Open robvdl opened 7 months ago
I would say this would probably make the configuration too complicated which is why I am not 100% for this myself.
Thanks for the point @robvdl - I definitely know where you're coming from as I recently faced this with Wagtail, too.
The problem is how would you define this in settings.py
The most obvious (to me) way to define it would be to switch to more of a lookup-dictionary pattern, where the key is a URL path:
# Standard, shared CSP config comes first
# ...
# CSP_IMG_SRC: "'self'"
# ...
# Then the exceptions/additionsl list
CSP_CONFIG_EXCEPTIONS = {
'default': {
"CSP_IMG_SRC": "'self' storage.example.com",
"CSP_SCRIPT_SRC": "'self'",
//... etc
},
'/admin/': {
"CSP_IMG_SRC": "'self' otherstorage.example.com",
"CSP_SCRIPT_SRC": "'self'",
//... etc
},
'/path/to/a/very/specific/page/': {
"CSP_IMG_SRC": "'self' otherstorage.example.com",
"CSP_SCRIPT_SRC": "'self'",
//... etc
},
}
This would be a supplementary setting and would merge in existing settings, so it'd be backwards compatible and only active if you set it.
Note: While the above would work for whole paths (and potentially individual pages), making that work with a wildcard would be trickier (but we could consider that scope creep and not suport that)
Thinking aloud: one problem with having a dynamic rule selection is that we'd have to re-evaluate it on every request, because the rules may differ on the path. I don't think this'll be a big deal computationally, but I haven't looked at whether this would need a big refactor to make happen. If the dictionary-based idea appeals, I can look into it more (or anyone can, really)
What do you think? I've not thought very deeply about this yet, and would welcome bouncing it around a bit.
That looks cool. To be honest I didn't think this would go anywhere so I had already resorted to subclassing the middleware and doing some of my own magic in there instead. It's not really up to me, but it looks like it could work at least.
Chatting with @robhudson he made the good point that it would be nice if third-party apps could automatically declare a (default) CSP for the pages they control. As such, a dict of settings isn't as automatically compatible with that - but it does make sense as a place for a site developer to have ultimate control over all CSP rules for given paths.
Worth us thinking about whether this behaviour can fit well into an AppConfig
for per-app defaults
Also, I do think there's some form of overlap here with https://github.com/mozilla/django-csp/issues/36 - maybe there's common ground in addressin both that Issue and this one
The problem with AppConfig, how can you control third party apps like Wagtail.
Anyway Wagtail are working on their CSP rules already, it seems they are closing in on not needing unsafe inline script-src and style-src.
Just wanted to flesh out the AppConfig idea for discussion. The idea would be to allow Django apps themselves to attempt to customize their CSP rules since they know best what limitations exist. The AppConfig seemed like a good place to store these.
This assumes there's already some data structure that can separate CSP configs by URL path that we would update if we find AppConfig CSP rules.
Here is an example. Let's say we have a blog app that has some inline javascript in the templates. That blog app would add a little bit of config to the AppConfig, e.g.
class BlogConfig(AppConfig):
name = "django_blog"
# Override CSP settings for this app.
# The keys in this config match the naming of the decorator keys (no `CSP_` prefix).
CSP_CONFIG = {
"SCRIPT_SRC": ["'unsafe-inline'"],
}
The django-csp app could then scan all app configs and look for a CSP_CONFIG
, and if found, append to the rules. Something like that might look like:
from django.apps import apps
for app, config in apps.app_configs.items():
if hasattr(config, "CSP_CONFIG"):
# Update the CSP rules
The tricky part is matching the app's URL prefix with what is set in the project's URL configuration. This can be done by comparing the config.module.urls
with the project's urlconf_module
from each URL pattern.
Extending the above, a pretty hacky way to match the urlconf_module
and pull out the prefix:
urlconf = importlib.import_module(settings.ROOT_URLCONF)
for app, config in apps.app_configs.items():
if hasattr(config, "CSP_CONFIG"):
for resolver in urlconf.urlpatterns:
if hasattr(resolver, "urlconf_module"):
if resolver.urlconf_module == config.module.urls:
prefix = resolver.pattern._route
At this point, all this data could be used to update the yet-to-be-written path-based CSP rules and 3rd-party Django apps (or django.contrib apps) can ship their own CSP configs.
The last comment is a bit of a complex solution.
Something that could possibly work now is to wrap the views inside the urls.py
with the @csp
decorator, providing the directives you want to override.
E.g.
from thirdparty.views import view1, view2
from csp.decorators import csp
custom_csp_policy = {
"DEFAULT_SRC": ["'self'"],
...
}
urlpatterns = [
path("view1/", csp(**custom_csp_policy)(view1)),
path("view2/", csp(**custom_csp_policy)(view2)),
]
This can work for 3rd party apps but can be a bit tedious. E.g. the django's contrib.auth has 8 views you would need to wrap and add to your urls.py
rather than using the include(...)
mechanism.
And the idea of a separate middleware that is keyed on path prefix to provide alternate policies would be a more simple solution than trying to map views<->paths automatically. We already provide a separate middleware for the report percentage setting. Providing one for path based overrides seems like a reasonable step.
Not sure if this will complicate things too much, but I want a stricter set of rules for my app's pages on / to rules for the /admin pages which in this case is Wagtail. Wagtail requires unsafe-inline and I cannot really control what that does very easily.
Right now from reading the django-csp source, I can set CSP_EXCLUDE_URL_PREFIXES and not do CSP headers for the /admin url, that is one way to go about it.
It would be much nicer to be able to set slightly looser set of rules for /admin instead of disabling CSP though.
The problem is how would you define this in settings.py