Opened 14 years ago

Last modified 5 days ago

#15727 assigned New feature

Add support for Content-Security-Policy (CSP) to core

Reported by: db.pub.mail@… Owned by: Rob Hudson
Component: HTTP handling Version: dev
Severity: Normal Keywords:
Cc: eromijn@…, emorley@…, Tom Forbes, Petr Přikryl, László Károlyi, Alexandr Artemyev, Adam Johnson Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

out of the box support for CSP would totally rock!
See https://developer.mozilla.org/en/Security/CSP/Introducing_Content_Security_Policy for more information on what CSP is about.

Change History (43)

comment:2 by Luke Plant, 14 years ago

Component: UncategorizedHTTP handling
Triage Stage: UnreviewedSomeday/Maybe

Having just read the specs now, it appears that for this to work you cannot have inline scripts. From looking at django-csp, it appears there is an option to re-enable inline scripts, but that would appear to seriously limit the usefulness of the protection.

The disabling of inline scripts is problematic for us in a number of ways:

  • Various places in the admin use inline scripts
  • Lots of Django projects will use the pattern of using {% url %} inside an inline javascript block in a template, even if the bulk of the javascript is in a separate file. There have been discussions recently about how to improve this, but no conclusion yet.

If a CSP middleware is not compatible with either our killer app or many third party apps, I'm afraid I don't share the enthusiasm of the reporter! It isn't going to get much use in Django, and there wouldn't be much point adding it. I should also note that due to the autoescaping feature of our template language, we have a pretty good record with XSS. (Not that additional security wouldn't be welcome, I'm just pointing out that it isn't a special priority for us).

To mark 'Accepted' would really imply that we are going to do something about the inline HTML in the admin and these other problems, which is unlikely in all honesty, so I'm going to mark as Someday/Maybe instead, although I'm also inclined towards Wontfix, because we need some compelling reason why this should be in core rather than as an external app like django-csp.

comment:3 by Luke Plant, 14 years ago

Type: New feature

comment:4 by Luke Plant, 14 years ago

Severity: Normal

comment:5 by d1b, 14 years ago

Well it would be a real nice to have. I sent an email reply but it was blocked :/
Django hasn't been 'xss free' and a new template tag could be added to transform inline js into js included and served from a given location(that the CSP policy allows).

Last edited 14 years ago by d1b (previous) (diff)

in reply to:  5 comment:6 by Luke Plant, 14 years ago

Replying to d1b:

a new template tag could be added to transform inline js into js included and served from a given location(that the CSP policy allows).

Sounds wonderful! Patch welcome :-)

(To explain what may not come over well on Trac: I'm implying that there are multiple significant problems with this suggestion that would need to be solved before it was practical. A solution to them isn't necessarily impossible, but will at least require quite a lot of work. What is written above is only one step away from saying "A new 'magic wand' feature could be added to Django that makes the problems with using CSP disappear, and then it would be sensible to add it". Indeed it would be...).

I sent an email reply but it was blocked :/

I don't know what you mean by an email reply - all emails sent from Trac are from the 'noreply@…' address.

comment:7 by Aymeric Augustin, 13 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:8 by Aymeric Augustin, 13 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:9 by Paul McMillan, 12 years ago

Triage Stage: Someday/MaybeAccepted

Now that CSP is much closer to being finalized, I would personally like to see this as part of Django core. We already protect against XSS, CSRF, and clickjacking; CSP fits right in with these features.

The work which can be done now (before the spec is finalized) mainly involves removing inline scripting and the mixing of styling and markup in the HTML templates we ship. This makes sense from the perspectives of both security and best practices. This would be a really good task for a group people to tackle during a sprint. Once the spec is finalized, we can work to integrate django-csp more closely, to the point that it makes sense to pull into core.

Even if we can't spend the time to really lock down the admin (and we may not, given our stance that the admin tends to be for trusted users), I think it makes sense to ship a CSP implementation with Django, so that projects can use a canonical, well tested, carefully implemented solution. Security features are hard to get right, and it makes sense to bless one and concentrate effort, rather than waiting to see which one wins.

The current spec can be found here: http://www.w3.org/TR/CSP/

I'm moving this back into accepted, with the caveat that it won't land in the near future (1.6 timeframe is probably realistic).

comment:10 by Sasha Romijn, 11 years ago

Cc: eromijn@… added
Summary: out of the box support for CSP would totally rock!Add support for Content-Security-Policy (CSP) to core

comment:11 by Nima, 10 years ago

After 3 years and 5 major releases, adding content security policy support to Django would still totally rock.

comment:12 by Rudolph Froger, 10 years ago

Owner: changed from nobody to Rudolph Froger
Status: newassigned

comment:13 by Rudolph Froger, 10 years ago

Has patch: set
Version: 1.2master

Pull request has been added with tests and documentation.

comment:14 by Sasha Romijn, 10 years ago

Needs documentation: set

Thanks for that patch. In general, I think it is important that we document well how to change projects to be CSP-compatible. The easier we make this, the more projects will use CSP, the safer people will be. A few quick notes:

  • I agree with PaulM that we could accept having the admin not CSP-compatible. However, we should very clearly document that CSP currently breaks with the admin. If I remember correctly, mozilla's django-csp allows one to add excluded paths, so that one can have both CSP and the admin enabled. That does reduce effectiveness, of course, but is better than no CSP.
  • With in-line javascript no longer allowed, it would be useful to point out <script type="application/json"></script> to people, which if my memory serves me right makes it easy to include a bit of json in your templates without violating CSP. Not 100% sure here though.
  • That can also help for the {% url %} problem described in comment:2, but perhaps there are other suggestions we can make.
  • This also deserves a place in the 1.8 release notes and in the security documentation (we have a specific page for that).

comment:15 by Rudolph Froger, 10 years ago

Thanks for your comment.

Current pull request does not enable a Content-Security-Policy by default, because we can make no assumptions about the implementation details of other people's code. So the admin will stay fully functional unless you specify a policy which is too strict for the admin. However I agree that it would be nice to also provide a default policy for the admin (as strict as currently possible), which should be configurable for people that want to allow more (i.e. load external scripts).

Comment:2 can be easily solved by putting url's and other data needed by Javascript in data attributes instead.

I'll add something to the release notes in my update to the pull request.

comment:16 by Carl Meyer, 10 years ago

For reference, the PR is at https://github.com/django/django/pull/3550

comment:17 by Carl Meyer, 10 years ago

The code looks reasonable to me. I agree with all of @erikr's recommendations regarding additional documentation.

comment:18 by Gavin Wahl, 10 years ago

I don't see any value in adding the ability for django to set the CSP header for you. I can easily do that myself with a middleware.

The important thing for django to support is to make the admin compatible with CSP. Otherwise, most installations can't use CSP anyway.

A builtin CSP middleware _could_ be useful if it had a standardized way of dealing with nonce an hash sources.

Version 1, edited 10 years ago by Gavin Wahl (previous) (next) (diff)

comment:19 by Tim Graham, 9 years ago

The work to remove inline JavaScript in the admin is tracked in #25165.

comment:20 by Tim Graham <timograham@…>, 9 years ago

In d638cdc:

Fixed #25165 -- Removed inline JavaScript from the admin.

This allows setting a Content-Security-Policy HTTP header
(refs #15727).

Special thanks to blighj, the original author of this patch.

comment:22 by Tim Graham, 9 years ago

Consensus from django-developers is to add this to SecurityMiddleware.

comment:23 by Ed Morley, 8 years ago

Cc: emorley@… added

comment:24 by Vlastimil Zíma, 8 years ago

If the django-csp should be included in Django. I suggest to modify it to allow enforcing and monitoring mode alongside, as noted in CSP specification itself. One set of rules may be enforced and different set of rules may be reported.

comment:25 by Tom Forbes, 6 years ago

Cc: Tom Forbes added
Owner: changed from Rudolph Froger to Tom Forbes

comment:26 by Sebastien Dubois, 6 years ago

Any news about this feature request?

comment:27 by Dylan Young, 5 years ago

FYI I've started work on multi policy support in django-csp. Once that's completed, it should be easy to fold into core.

comment:28 by Collin Anderson, 3 years ago

https://github.com/mozilla/django-csp/issues/135 is the tracking ticket for merging on the django-csp side of things. Sounds like we want to have support for Report-Only / multiple policies before merging.

comment:29 by Dylan Young, 3 years ago

One potential snag here is that I don't think Django can currently support multiple of the same header currently (aside from Set-Cookie), unless there's some API I'm missing...

Is this something there's interest in adding to core? It looks like the python native wsgiref supports this as well: https://docs.python.org/3/library/wsgiref.html?highlight=headers%20multi%20value#module-wsgiref.headers.

See here:

https://github.com/w3c/webappsec-csp/issues/215#:~:text=A%20server%20MUST%20NOT%20send,resource%20or%20with%20different%20resources.

I don't think it's critical, but it'd be nice as it's standards compliant behaviour.

comment:30 by Anvesh Mishra, 22 months ago

Owner: changed from Tom Forbes to Anvesh Mishra

I have summarized the state of play so far here and will be working on this ticket.

comment:31 by Claude Paroz, 22 months ago

Also note #25706 (CSP for GIS admin) is still open and is a non-trivial issue.

comment:32 by Petr Přikryl, 22 months ago

Cc: Petr Přikryl added

comment:33 by Anvesh Mishra, 22 months ago

Had some implementation ideas needed suggestions:
CSP should be added to the SecurityMiddleware according to Tim Graham's comment, so some of the implementation ideas that I wanted to share are:
1) The following settings will have to be added to conf\global_settings.py:

SECURE_CSP = {}
SECURE_CSP_INCLUDE_NONCE_IN = None
SECURE_CSP_REPORT_ONLY = {}
SECURE_CSP_EXCLUDE_URL_PREFIXES = ()

2) Implementation for CSP and Report-Only with nonce support in SecurityMiddleware:

class SecurityMiddleware(MiddlewareMixin):
    def __init__(self, get_response):
        super().__init__(get_response)
      
        self.csp = settings.SECURE_CSP
        self.csp_report_only = settings.SECURE_CSP_REPORT_ONLY
        self.csp_nonce = settings.SECURE_CSP_INCLUDE_NONCE_IN

    def _make_nonce(self, request):
        if not getattr(request, '_csp_nonce', None):
            request._csp_nonce = (
                base64
                .b64encode(os.urandom(16))
                .decode("ascii")
            )
        return request._csp_nonce

    def process_request(self, request):
        path = request.path.lstrip("/")
        nonce = partial(self._make_nonce, request)
        request.csp_nonce = SimpleLazyObject(nonce)
        if (
            self.redirect
            and not request.is_secure()
            and not any(pattern.search(path) for pattern in self.redirect_exempt)
        ):
            host = self.redirect_host or request.get_host()
            return HttpResponsePermanentRedirect(
                "https://%s%s" % (host, request.get_full_path())
            )

    def process_response(self, request, response):
        .................................
        if self.csp:
            csp_header = '; '.join(
                (f'{k} {v}' for k, v in self.csp.items())
            )
            if self.csp_nonce:
                nonce = getattr(request, '_csp_nonce', None)
                csp_header += "; 'nonce-%s'" % nonce
            response.headers["Content-Security-Policy"] = csp_header
        if self.csp_report_only:
            csp_header = '; '.join(
                (f'{k} {v}' for k, v in self.csp_report_only.items())
            )
            response.headers["Content-Security-Policy-Report-Only"] = csp_header
        return response

The CSP and Report-Only are repetitive so will making something like SecurityMiddleware.csp_policy_builder() be apt?
NOTE: This implementation is not the actual representation of the overall implementation it's just a snippet
3) The CSP nonce context processor:

def nonce(request):
    nonce = request.csp_nonce if hasattr(request, 'csp_nonce') else ''

    return {
        'CSP_NONCE': nonce
    }
Last edited 22 months ago by Anvesh Mishra (previous) (diff)

comment:34 by Anvesh Mishra, 19 months ago

Submitted a WIP PR. Open for suggestions.

comment:35 by Rob Hudson, 7 months ago

Hi!

I submitted a draft PR to add CSP support to Django.
https://github.com/django/django/pull/18215

As the current maintainer of django-csp, I'd love to help get CSP support in Django using django-csp as a reference or a guide.

comment:36 by Sarah Boyce, 4 months ago

#35692 was was marked as a duplicate, as work can be tracked as part of this ticket

comment:37 by László Károlyi, 4 months ago

Cc: László Károlyi added

comment:38 by Alexandr Artemyev, 4 months ago

Cc: Alexandr Artemyev added

comment:39 by Rob Hudson, 4 months ago

Owner: changed from Anvesh Mishra to Rob Hudson

comment:40 by Natalia Bidart, 4 months ago

Cc: Adam Johnson added

Adam, I took the liberty to add you as cc since I figured you may be interested in providing feedback for the current/active WIP PR. Thanks!

comment:41 by Rob Hudson, 4 weeks ago

https://github.com/django/django/pull/18215 is considered ready for review.

comment:42 by Carlton Gibson, 7 days ago

Needs documentation: unset

Updated the flags so the ticket appears ready for review, in line with Rob’s comment.

comment:43 by Sarah Boyce, 5 days ago

Patch needs improvement: set
Note: See TracTickets for help on using tickets.
Back to Top