nelmio / NelmioSecurityBundle

Adds extra security-related features in your Symfony application
https://symfony.com/bundles/NelmioSecurityBundle/
MIT License
650 stars 85 forks source link

Is it posible to override csp for specified route? #36

Closed hd-deman closed 8 years ago

Seldaek commented 9 years ago

Not at the moment the settings are only global for the application as enabling this everywhere has a lot more value than if you start allowing holes here and there. If you really want though it could be extended to support by-path configuration like other features have. In case you want to work on it it would be best to wait until #33 is merged.

thkoch2001 commented 9 years ago

I'm curious to learn the use case for this feature request and whether overriding some routes would be the best solution. As a hack it might be possible to add another listener that manipulates the response after the CSP header were set and removes them again for a given set of routes.

hd-deman commented 9 years ago

Yes, I think overriding some routes would be the best solution.

ghost commented 9 years ago

i have a use case for this feature! The WebProfilerBundle can be used on live sites, especially since one can use request matchers to provide profiles for specific requests/paths/etc.

However, it is impossible for the WebProfilerBundle to work with CSP setups that are close to being strict due to all the inline styles, assets, and scripts.

ghost commented 9 years ago

I was just about to add a whiteList parameter to the ContentSecurityPolicyListener. I've already added the appropriate tests as well. Should something different be done instead?

Seldaek commented 9 years ago

I think supporting path-based config would be most flexible because you can then have different strictness levels on different sections of the app, but a whitelist would be an easier solution to implement for sure.. Up to you but if you have time for a fully fledged version (i.e. what clickjacking does) that'd be great.

ghost commented 9 years ago

can you do a bit of a snippet of how you think it might look under the bundle configuration (as yaml). tryin to think about how it might work with the various configurations of report, enforce and how default-src is the default for everything.

ghost commented 9 years ago

Why did you move to enforce and report being top level nodes instead of options of default-src,frame-src, etc anyways?

Seldaek commented 9 years ago

The idea is just to have two configs, one for the main CSP header and one for the report-only header.

From this:

nelmio_security:
    csp:
        report_logger_service: logger
        enforce:
            report-uri: /nelmio/csp/report
            default-src: [ 'self' ]
            frame-src: [ 'https://www.youtube.com' ]
            script-src:
                - 'self'
                - 'unsafe-inline'
            img-src:
                - 'self'
                - facebook.com
                - flickr.com
        report:
            report-uri: /nelmio/csp/report
            script-src:
                - 'self'

We could go to:

nelmio_security:
    csp:
        report_logger_service: logger
        paths:
            - ^/foo:
                enforce:
                    report-uri: /nelmio/csp/report
                    default-src: [ 'self' ]
                    frame-src: [ 'https://www.youtube.com' ]
                    script-src:
                        - 'self'
                        - 'unsafe-inline'
                    img-src:
                        - 'self'
                        - facebook.com
                        - flickr.com
                report:
                    report-uri: /nelmio/csp/report
                    script-src:
                        - 'self'
            - ^/:
                report:
                    report-uri: /nelmio/csp/report
                    script-src:
                        - 'self'

So you could define rules per path, and you could either define report/enforce OR paths at the top level.

If you see a better way I am fully open to suggestions :)

ghost commented 9 years ago

I'm just trying to make sure it's not awkard to be able to apply the same config to multiple paths as such that i can say both ^/_wdt and ^/_profiler get the same configuration without duplicating it for each.

I'd prefer not to try to encompass it all in a single regex, but i guess we could:

paths: '^/_(wdt|profiler)'
  enforce:
    # ...

vs:

paths:
  - '[^/_wdt', '^/_profiler']
    enforce:
      # ...

which feels a little weird to me, but maybe it's not not as bad as i think it is.

EDIT: I basically wanna avoid the header totally in those situations, so maybe enforce and report would both be set to null/empty in that configuration.

ghost commented 9 years ago

If we can come up with an agreement about this, then we'd end up with 3 configs to parse, the new one, the current one, and the legacy one.

What should be done about that? should we bump the major version and drop both other configs, or just drop the (current) "legacy" config.

thkoch2001 commented 9 years ago

Just leave report and enforce as they are now and add path at the same level. If a route matches a defined path than it takes the config from there. Otherwise the default config from the 'global' report and enforce settings is applied.

nelmio_security:
    csp:
        report_logger_service: logger
        report:
            report-uri: /nelmio/csp/report
            script-src:
                - 'self'
        enforce:
            report-uri: /nelmio/csp/report
            img-src:
                - 'self'
                - facebook.com
        paths:
            - ^/foo:
                enforce:
                    report-uri: /nelmio/csp/report
                    default-src: [ 'self' ]
                    frame-src: [ 'https://www.youtube.com' ]
                report:
                    report-uri: /nelmio/csp/report
                    script-src:
                        - 'self'
ghost commented 9 years ago

so we're assuming that the top level settings csp settings are '^/' then.

thkoch2001 commented 9 years ago

Coming back to your initial requirement: I still don't think that it's a good idea to have different CSP settings for different paths. One should aim to eliminate all CSP "bugs" in a site and make the CSP headers as strict as possible.

And with @jrobeson 's proposal it is still not possible to embed the symfony toolbar in sites that don't allow inline styles/scripts/assets since the CSP checks are done for the site that embeds the toolbar and not the paths of the toolbar styles/scripts/assets.

@jrobeson please open an issue in the symfony project whether it is possible (why not?) to make the toolbar CSP compliant.

ghost commented 9 years ago

@thkoch2001 : not gonna happen, they merged it all together to make it easier to be embedded in silex and other projects that use an HttpKernelInterface. It's totally on purpose and not going to change.

EDIT: the only thing they could do is start adding script-nonces and stuff like that, but i don't really expect them to keep up with that.

ghost commented 9 years ago

@thkoch2001 : i've been able to make it work by overriding somemplates and then allowing inline-styles and data uris, i'm ok with that level of support currently I'll probably try to make it work without the inline styles someday, but i'm mostly concerned about disallowing inline scripts.

ghost commented 9 years ago

To do path based overrides would require a fundamental restructuring of the listener itself and probably DirectiveSet too. We'd end up having a collection of DirectiveSets ordered by paths instead.

maybe something like

new DirectiveSet('enforce|report', '/path');

Seldaek commented 9 years ago

@jrobeson as I said above (or other issue not sure anymore) a whitelist is a good cheaper alternative if this is too much work..

romainneutron commented 8 years ago

This issue has been solved in https://github.com/nelmio/NelmioSecurityBundle/pull/74

Here's an example of implementation:

<?php

namespace MyApp\EventListener;

use Nelmio\SecurityBundle\EventListener\ContentSecurityPolicyListener;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;

class CSPEventSubscriber implements EventSubscriberInterface
{
    private $listener;

    public static function getSubscribedEvents()
    {
        return [
            // must be higher than 0 (before the bundle listener)
            KernelEvents::RESPONSE => ['onKernelResponse', 5],
        ];
    }

    public function __construct(ContentSecurityPolicyListener $listener)
    {
        $this->listener = $listener;
    }

    public function onKernelResponse(FilterResponseEvent $event)
    {
        if (!$event->isMasterRequest()) {
            return;
        }

        $request = $event->getRequest();

        // If URL begins with "/admin/"
        if (0 !== strpos($request->getPathInfo(), '/admin/')) {
            return;
        }

        $directive = $this->listener->getEnforcement()->getDirective('img-src');
        // add "https://domain.com" as a valid source for images
        $this->listener->getEnforcement()->setDirective('img-src', ltrim($directive.' https://domain.com'));
    }
}
ghost commented 8 years ago

@romainneutron : I don't see how this stops the regular CSP listener from doing what it would normally do.

romainneutron commented 8 years ago

It does not stop, it allows to override a directive at runtime. Isn't it the topic of this thread?