search

IdentityPython / djangosaml2

Django SAML2 Service Provider based on pySAML2
Apache License 2.0
254 stars 143 forks source link

Fix CSP-Uses where hash-values are already specified for script-src #403

Open prauscher opened 2 months ago

prauscher commented 2 months ago

Hello,

I just figured out that using csp_update can result in a problem: If you (for some reason) specified a hash in the global CSP-Configuration for script-src, the introduced 'unsafe-inline' gets ignored. There are two possible options in my mind:

  1. use csp_replace instead of csp_update to ignore the CSP-Header specified by the integrating project. Would be a quick fix, but does not feel too good imho.
  2. replace the form-template to use a nonce - but the template is by default taken from PySAML2: https://github.com/IdentityPython/pysaml2/blob/7cb4f09dce87a7e8098b9c7552ebab8bc77bc896/src/saml2/pack.py#L38

Imho the best solution would be to include a default post_binding_form.html which uses a nonce. This would also remove the required 'unsafe-inline' from CSP-Settings. What are your feelings regarding this? And should this be a new PR or shall it be integrated to #401? My suggestion would be to include it in #401 iff option 1 would be selected, but a separate if option 2 would be selected.

peppelinux commented 2 months ago

Ciao @prauscher

the PR number 401 is now merged. I'd go for another PR and in addition to this I would appreciate if you could also advance the version of the release in the file setup.py

We'll produce a new release after the next PR about this CSP

prauscher commented 2 months ago

Thanks for the fast feedback, I opted for number 2, so see #404 :)

prauscher commented 2 months ago

Just to keep it noted here: #404 is now merged, but only solves this issue for login-requests by providing a post binding form which uses the nonce. The same would be required for logout, but currently djangosaml2 does not use a post binding template for logout, but recycles the html received from pysaml2.

So if you are using hashes in your script-src-option of csp and use logout with post bindings, you will still have the problem of Content-Security-Policy not matching during logout, giving you a warning about 'unsafe-inline' being ignored due to hashes while hashes are in place.

To fix this properly, djangosaml2 would require a overhaul to use own templates during logout too. In the meantime, you could probably use a SAML_CSP_HANDLER introduced in #401 to use csp_replace instead of csp_update.

peppelinux commented 2 weeks ago

@prauscher is there actionable to resolve this issue? Is there any hints to introduce some improvements beyond pr 401, or is somethign that you think that coud handled only with some good documentation?

prauscher commented 1 week ago

I would leave this issue open to document the problem: During POST-Logout, djangosaml2 does not specify its own template but instead relies on the template specified by pysaml2. As this only affects logout, it is probably not too much of an issue. To be fixed, one would need to copy the templating-logic of login-pages to logout-pages and provide a matching template (could probably be copied too).