Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support different sets of rules for paths like /admin #203

Open
robvdl opened this issue Jan 12, 2024 · 8 comments
Open

Support different sets of rules for paths like /admin #203

robvdl opened this issue Jan 12, 2024 · 8 comments
Labels
feature new features

Comments

@robvdl
Copy link

robvdl commented Jan 12, 2024

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

@robvdl
Copy link
Author

robvdl commented Jan 12, 2024

I would say this would probably make the configuration too complicated which is why I am not 100% for this myself.

@stevejalim
Copy link
Contributor

stevejalim commented Jan 24, 2024

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.

@robvdl
Copy link
Author

robvdl commented Jan 24, 2024

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.

@stevejalim stevejalim added the feature new features label Jan 24, 2024
@stevejalim
Copy link
Contributor

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

@stevejalim
Copy link
Contributor

Also, I do think there's some form of overlap here with #36 - maybe there's common ground in addressin both that Issue and this one

@robvdl
Copy link
Author

robvdl commented Jan 25, 2024

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.

@robhudson
Copy link
Member

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.

@robhudson
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature new features
Projects
None yet
Development

No branches or pull requests

3 participants