Post-response tools needing access to csp_nonce after view generates response

[tim-schilling]

Hi folks, we're running into an issue with the django debug toolbar (#2082) with the feature implemented in #247 when trying to render extra content after the view has processed for the debug toolbar. Would it be possible for this to continue to raise the error, but also expose the nonce if it's caught? That way a person can decide that they need access to the original nonce or not.

The toolbar injects extra content into the response after the view has processed the request, so disabling the nonce at that point prevents the toolbar from adding extra static content to the response.

If the toolbar shouldn't be doing this, then I may need some help on designing how this should work.

[robhudson]

I will be able to try to reproduce this in a couple days and think through some options and share back here. Thanks for the report!

[tim-schilling]

I'm now realizing the toolbar middleware could grab this from the request on pre view side. Though it would mean the csp middleware would need to be first. I'm not sure how I feel about that as a long term solution.

[jwhitlock]

The reason it is done this way is that the post-response code has to add the nonce to the CSP header, so if you "generate" the nonce afterwards, the content is still blocked because there is no nonce in the CSP header.

You could write a new attribute, like request.middleware_needs_csp_nonce = True, in your pre-response code, and the CSP could use that to unconditionally generate the nonce and add it to the header.

[tim-schilling]

The reason it is done this way is that the post-response code has to add the nonce to the CSP header, so if you "generate" the nonce afterwards, the content is still blocked because there is no nonce in the CSP header.

Gotcha that makes sense. Thank you for the clarification.

You could write a new attribute, like request.middleware_needs_csp_nonce = True, in your pre-response code, and the CSP could use that to unconditionally generate the nonce and add it to the header.

I'm not following what you're suggesting here. Do you mind elaborating a bit? Please keep in mind, this is for another library. I wouldn't have control of the views in this scenario.

[jwhitlock]

What I was thinking is that django-debug-toolbar would change this code:

https://github.com/django-commons/django-debug-toolbar/blob/19bb9aafe6ac530bc4c9219fe79d71cc4f5f56cd/debug_toolbar/middleware.py#L86-L91

to something like:

class DebugToolbarMiddleware:
    ...

    def __call__(self, request):
        ...
        # Decide whether the toolbar is active for this request.
        show_toolbar = get_show_toolbar()
        if not show_toolbar(request) or DebugToolbar.is_toolbar_request(request):
            return self.get_response(request)
        
        # Require CSP nonce
        request.middleware_requires_csp_nonce = True

        toolbar = DebugToolbar(request, self.get_response)
        ...

and then django-csp would read it here

class CSPMiddleware(MiddlewareMixin):
    ....

    def process_response(self, request: HttpRequest, response: HttpResponseBase) -> HttpResponseBase:
       ...
        if response.status_code in exempted_debug_codes and settings.DEBUG:
            return response

        # Check if other middleware needs the nonce
        if getattr(request, 'middleware_requires_csp_nonce', False):
            self._make_nonce()

        policy_parts = self.get_policy_parts(request=request, response=response)
        csp = build_policy(**asdict(policy_parts))

        # Once we've written the header, accessing the `request.csp_nonce` will no longer trigger
        # the nonce to be added to the header. Instead we throw an error here to catch this since
        # this has security implications.
        if not getattr(request, 'middleware_requires_csp_nonce', False):
            setattr(request, "csp_nonce", SimpleLazyObject(self._csp_nonce_post_response))

but in writing it out, I'm thinking of other changes:

• If the nonce was initialized, we should leave request.csp_nonce set to the value. This will let the django debug toolbar still access it
• If the nonce was not initialized, then bool(request.csp_nonce) should return False, and str(request.csp_nonce) should raise an error. I'm not 100% sure this would work in Python

I think there is still needed some way to communicate that DebugToolbarMiddleware needs the csp nonce. One way would be a simple middleware, like:

def init_csp_nonce_middleware(get_response):
    def middleware(request):
        getattr(request, "csp_nonce", None)
        return get_response(request)
    return middleware

and then the django debug toolbar tests become:

    @override_settings(
        MIDDLEWARE=settings.MIDDLEWARE + ["csp.middleware.CSPMiddleware", "init_csp_nonce_middleware"]
    )
    def test_exists(self):
    ...

This seems better than a magic cross-project attribute. It still needs this project to leave request.csp_nonce readable if set.

[jwhitlock]

I've implemented my idea in PR #269, take a look. The django-debug-toolbar tests will still need something like init_csp_nonce_middleware.

[tim-schilling]

I think it might actually be best if the toolbar relies on request._csp_nonce rather than the lazy object. There's too much magic in the lazy evaluation combined with the 4 scenarios (middleware order, is the nonce accessed in the view).

Relevant PR: django-commons/django-debug-toolbar#2088

[jwhitlock]

I'd rather you didn't use private members, that feels like a recipe for future breakage in bizarre ways.