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

Rely on django-csp's private attribute for nonce #2088

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

tim-schilling
Copy link
Member

Description

This refactors how the CSP nonce is fetched. It's now done as a toolbar property and wraps the private attribute request._csp_nonce

This avoids the toolbar from generating a nonce that gets injected into the CSP header when the view doesn't expect it to. It also supports using a nonce that is generated from any other point while processing the request, including other middleware.

Fixes #2082

Checklist:

  • I have added the relevant tests for this change.
  • I have added an item to the Pending section of docs/changes.rst.

This refactors how the CSP nonce is fetched. It's now done as
a toolbar property and wraps the private attribute request._csp_nonce

This avoids the toolbar from generating a nonce that gets injected
into the CSP header when the view doesn't expect it to. It also
supports using a nonce that is generated from any other point
while processing the request, including other middleware.
@@ -42,6 +42,11 @@ def regular_view(request, title):
return render(request, "basic.html", {"title": title})


def csp_view(request):
"""Use request.csp_nonce to inject it into the headers"""
return render(request, "basic.html", {"title": f"CSP {request.csp_nonce}"})
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The nonce needed to be rendered in the view. I chose to do it here.

Comment on lines +75 to +76
for middleware in [MIDDLEWARE_CSP_BEFORE, MIDDLEWARE_CSP_LAST]:
with self.settings(MIDDLEWARE=middleware):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It become important to test with both configurations of middleware. Or at least it will be more important if we stop relying on request._csp_nonce

@robhudson
Copy link
Contributor

The latest commit on django-csp changes the way nonce access is handled that may help this situation and not have to use the private attribute.

If the nonce was used in the content it will still be available after the middleware has processed the response so other middleware can reference it.

If the nonce was NOT used in the content it will raise the error, but also checking the nonce via if request.csp_nonce will return False.

You can see the changes in the PR: mozilla/django-csp#269

@tim-schilling
Copy link
Member Author

The lazy object solution may not work for the toolbar in the case where CSPMiddleware appears before DebugToolbarMiddleware and the view doesn't use request.csp_nonce.

  1. CSPMiddleware pre-view, configures the lazy request.csp_nonce
  2. DebugToolbarMiddleware pre-view, does nothing relevant
  3. View doesn't use request.csp_nonce, but has static assets in the rendered template
  4. DebugToolbarMiddleware post view, finds request.csp_nonce as truthy, generates a nonce and uses it for the toolbar's static assets
  5. CSPMiddleware sees that request.csp_nonce has been used and includes it in the header

This seems like the CSP for this response would include the nonce in the header, on the toolbar assets, but not the user's own static assets. I believe that will violate the CSP and cause things to not load/run properly. Please correct me if I'm wrong here. CSP isn't something I'm great with.

I'd like to avoid having to dictate what order these middleware need to appear in. Though that may be me trying to have my cake and eat it too.

@jwhitlock
Copy link

OK, what if in step 4 request.csp_nonce is falsy? Would that work for you?

  1. CSPMiddleware pre-view, configures the lazy request.csp_nonce with FalseLazyObject
  2. DebugToolbarMiddleware pre-view, does nothing relevant
  3. View doesn't use request.csp_nonce, but has static assets in the rendered template
  4. DebugToolbarMiddleware post view, finds request.csp_nonce as falsly, does not generate a nonce or use it for the toolbar's static assets
  5. CSPMiddleware sees that request.csp_nonce is still unset and omits it from the header

@tim-schilling
Copy link
Member Author

@jwhitlock yup, yup. That would work well. A read-only csp_nonce is what the toolbar needs and why relying on the private attribute worked. I don't know how much of django-csp's API should be catered to this particular problem. I do appreciate the efforts you've gone to already.

@jwhitlock
Copy link

Thanks, I should be able to get that PR open today or tomorrow.

Until then, maybe the solution is to exclude django-csp 4.0b4? It seems 4.0b3 was OK, and hopefully 4.0b5 will as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix failing tests in CspRenderingTestCase
3 participants