Opened 14 years ago
Last modified 5 days ago
#15727 assigned New feature
Add support for Content-Security-Policy (CSP) to core
Reported by: | 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:1 by , 14 years ago
comment:2 by , 14 years ago
Component: | Uncategorized → HTTP handling |
---|---|
Triage Stage: | Unreviewed → Someday/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 , 14 years ago
Type: | → New feature |
---|
comment:4 by , 14 years ago
Severity: | → Normal |
---|
follow-up: 6 comment:5 by , 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).
comment:6 by , 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:9 by , 12 years ago
Triage Stage: | Someday/Maybe → Accepted |
---|
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 , 11 years ago
Cc: | added |
---|---|
Summary: | out of the box support for CSP would totally rock! → Add support for Content-Security-Policy (CSP) to core |
comment:11 by , 10 years ago
After 3 years and 5 major releases, adding content security policy support to Django would still totally rock.
comment:12 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:13 by , 10 years ago
Has patch: | set |
---|---|
Version: | 1.2 → master |
Pull request has been added with tests and documentation.
comment:14 by , 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 , 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 , 10 years ago
For reference, the PR is at https://github.com/django/django/pull/3550
comment:17 by , 10 years ago
The code looks reasonable to me. I agree with all of @erikr's recommendations regarding additional documentation.
comment:18 by , 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. This really seems like a third-party app and not something in django core.
comment:23 by , 8 years ago
Cc: | added |
---|
comment:24 by , 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 , 6 years ago
Cc: | added |
---|---|
Owner: | changed from | to
comment:27 by , 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 , 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 , 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:
I don't think it's critical, but it'd be nice as it's standards compliant behaviour.
comment:30 by , 22 months ago
Owner: | changed from | to
---|
I have summarized the state of play so far here and will be working on this ticket.
comment:31 by , 22 months ago
Also note #25706 (CSP for GIS admin) is still open and is a non-trivial issue.
comment:32 by , 22 months ago
Cc: | added |
---|
comment:33 by , 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?
Also 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 }
comment:35 by , 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 , 4 months ago
#35692 was was marked as a duplicate, as work can be tracked as part of this ticket
comment:37 by , 4 months ago
Cc: | added |
---|
comment:38 by , 4 months ago
Cc: | added |
---|
comment:39 by , 4 months ago
Owner: | changed from | to
---|
comment:40 by , 4 months ago
Cc: | 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 , 4 weeks ago
https://github.com/django/django/pull/18215 is considered ready for review.
comment:42 by , 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 , 5 days ago
Patch needs improvement: | set |
---|
FTR, there is https://github.com/mozilla/django-csp