Footgun when accessing nonce value in a middleware

[alynn-coefficient]

django-csp only generates the nonce value (and includes it the header) the first time that .csp_nonce is accessed on a request. However, if that's accessed first in a middleware after django-csp has completed, it'll be generated but not included in the header, because django-csp has already finished processing and sendings its headers. Slightly more concretely, if we have:

MIDDLEWARE = [
    ...
    "foo.bar",
    "csp.middleware.CSPMiddleware",
    ...
]

And foo looks something like:

def bar(get_response):
    def middleware(request):
        response = get_response(request)
        response = inject_script_into_response(response, nonce=request.csp_nonce)
        return response
    return middleware

The script will correctly have a CSP nonce in it, but the CSP header won't necessarily include the nonce because of the position of CSPMiddleware.

Obviously the fix is on the user here and "foo.bar" should be positioned after the CSP middleware, however I can attest this took a while to track down. I think it would be better to make this a hard error if a CSP nonce doesn't already exist.

[robhudson]

The CSP middleware hangs the nonce off of request, but I see in the example you're using the response. Is that a typo?

    response = inject_script_into_response(response, nonce=response.csp_nonce)

The nonce is wrapped up in a SimpleLazyObject with the intention that if it is not accessed during the request/response cycle it won't be added to the header (usually in template code where it gets rendered). The decision around this pre-dates me and I'm curious what led to it, but that's how the code is set up to work. In tests we simulate this by coercing it to a str to trigger the lazy object, e.g. str(request.csp_nonce), so that may be something to try if swapping to request from response is a typo.

[alynn-coefficient]

Apologies! Yes, that's a typo. I'll amend the original example.

I have no opinion on whether the lazy object approach is good or bad, just that this quite specific case where a user misuses the system gives a surprising result, and I think a clear error message would be better.

[robhudson]

Can you try:

        response = inject_script_into_response(response, nonce=str(request.csp_nonce))

to see if the str coercion works? If your middleware is before the CSP middleware, it should trigger the nonce to be added to the headers, similar to accessing the nonce via templates during the req/resp cycle.

[alynn-coefficient]

So here's a project which reproduces the issue: https://github.com/alynn-coefficient/django-csp-247

Observe that:

• With django_csp_247.middleware.csp_using_middleware placed before csp.middleware.CSPMiddleware in MIDDLEWARE, we have a <script> tag included on / with a nonce value but the corresponding nonce value is not included in the Content-Security-Policy header. Verify this by curling / from runserver, or running the tests. I think in this case django-csp should either raise an error or include the nonce in the CSP header, probably the former.
• With django_csp_247.middleware.csp_using_middleware placed after csp.middleware.CSPMiddleware in MIDDLEWARE, everything works as expected.

[robhudson]

The app was illuminating on the details of the initial report, thank you for putting that together.

I'll summarize what you've already shared from the start...

Quoting the Django docs:

You can think of it like an onion: each middleware class is a “layer” that wraps the view, which is in the core of the onion. If the request passes through all the layers of the onion (each one calls get_response to pass the request in to the next layer), all the way to the view at the core, the response will then pass through every layer (in reverse order) on the way back out.

django-csp middleware before app middleware

With the app's middleware after django-csp's middleware, we get:
Request -> csp[1] -> app -> view -> app[2] -> csp[3] -> Response

1. django-csp sets up the request.csp_nonce SimpleLazyObject
2. app injects the nonce into the response
3. django-csp builds the CSP header and since request.csp_nonce has been accessed, it adds the nonce to the header.

django-csp middleware after app middleware

When the django-csp middleware is after the app's middleware, we get:
Request -> app -> csp[1] -> view -> csp[2] -> app[3] -> Response

1. django-csp sets up the request.csp_nonce SimpleLazyObject
2. django-csp builds the CSP header and since request.csp_nonce hasn't been accessed yet, it does not add the nonce to the header.
3. app injects the nonce into the response

Options

Documentation: This seems like something worth noting in the docs, for sure. It's subtle and can cause a lot of time spent wondering why the nonce isn't showing up.

You mention:

I think in this case django-csp should either raise an error or include the nonce in the CSP header, probably the former.

but according to django-csp's middleware point of view, it has accomplished it's goal. The nonce was never accessed within it's scope of the request/response cycle so it didn't include the nonce in the header. Only after django-csp added the header was the nonce injected into the body of the HTML.

Raise an error: It would be possible to swap out the request.csp_nonce on the way back out to the response so that if it is accessed again it raises an error. This seems a little dangerous to me -- I'd be curious if there may be cases where general logging or tracing is happening that could trigger this error. Perhaps logging a warning. Or an error only if settings.DEBUG is true.

Include the nonce in the header: I can think on this more but on initial thought I don't see how this is possible. The req/resp cycle has already passed through the django-csp code and the header added.

Always include the nonce in the header, whether accessed during the req/resp cycle or not: I think there are valid reasons to avoid adding the nonce to the header if it is not used:

• misleading: When you see a nonce you assume it is being used in content
• security: probably not a concern here, but exposing the nonce when not needed could potentially increase the attack surface
• caching: downstream caches would consider each response unique with a per-request nonce, making each cache request a miss

That's all the options I can think of at the moment. I'm open to ideas and suggestions. I appreciate that this is being highlighted and reconsidered.

[alynn-coefficient]

I think it would be possible to add the header after the fact if you altered the SimpleLazyObject to keep a reference to the response on the way out, but that also seems very weird and not worth it to me.

[jwhitlock]

I think it would be possible to add the header after the fact if you altered the SimpleLazyObject to keep a reference to the response on the way out, but that also seems very weird and not worth it to me.

This would required recreating the CSP header if someone accesses the nonce after the header was created. But that's the main point of CSPMiddleware. I guess it would make sense if you split the middleware into CSPMiddlewareSetupNonce and CSPMiddlewareInjectHeader, and made your stack:

MIDDLEWARE = [
    ...
    "CSPMiddlewareInjectHeader",
    "foo.bar",
    "CSPMiddlewareSetupNonce",
    ...
]

You could certainly create CSPMiddlewareInjectHeader and CSPMiddlewareSetupNonce for yourself by deriving them from csp.middleware.CSPMiddleware, but I wouldn't want to force that solution for all users.

I think if you've started writing your own Django middleware, then you need to know about middleware order and operations.

I'm in favor of documenting it, something like:

csp.middleware.CSPMiddleware should be placed in MIDDLEWARE above any other middlewares that use the CSP nonce.

and then raising the error if the csp_nonce is accessed after headers are written, something like:

   def used_csp_nonce_too_late():
      raise AttributeError("csp_nonce is not available after the CSP header is written. Maybe change your MIDDLEWARE order?")
    setattr(request, "csp_nonce", SimpleLazyObject(used_csp_nonce_too_late))

I wouldn't mess with changing this with DEBUG, since CSP stuff is most important in production.

[alynn-coefficient]

Completely on board with that as a solution :)