Skip to content

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

Open

Description

@prauscher

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.

Activity

  1. peppelinux commented on Apr 30, 2024

    @peppelinux
    Member

    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

  2. prauscher commented on Apr 30, 2024

    @prauscher
    ContributorAuthor

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

  3. prauscher commented on May 3, 2024

    @prauscher
    ContributorAuthor

    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.

  4. peppelinux commented on Jun 19, 2024

    @peppelinux
    Member

    @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?

  5. prauscher commented on Jun 28, 2024

    @prauscher
    ContributorAuthor

    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).

  6. prauscher commented on Jan 24, 2025

    @prauscher
    ContributorAuthor

    Just as remark for anyone trying to resolve this using SAML_CSP_HANDLER:

    SAML_CSP_HANDLER = "governor.utils.saml_csp_handler"

    and as a separate file (referenced in SAML_CSP_HANDLER, so here project/utils.py)

    SAML_CSP_DECORATOR = csp_replace(SCRIPT_SRC=["'unsafe-inline'"],
                                     FORM_ACTION=["https:"])
    
    def saml_csp_handler(response: HttpResponse) -> HttpResponse:
        # csp_replace is a decorator for a view, but we only get the final response
        # so we create a "dummy-view" and call the decorated dummy-view immediatly
        return SAML_CSP_DECORATOR(lambda: response)()

    Note that SAML_CSP_HANDLER does not support a Callable, so a separate file is required.

    Also note that django-csp==4.0 is just around the corner and will introduce some breaking api-changes, so ymmv.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions