Modify content security policy

[Henni]

fixes #47

probably breaks compatibility for owncloud < 8.1

[DJaeger]

Because of the compatibility break I have asked before I have created a PR. If a legacy version should be preserved, another branch should be created before.

[DJaeger]

For fixing

Content Security Policy: Die Einstellungen der Seite haben das Laden einer Ressource auf self blockiert ("script-src https://my.url 'unsafe-eval'").
onsubmit attribute on DIV element

I think you have to also add

        $csp->addAllowedScriptDomain('\'self\'');

[brantje]

Nice, thanks for your contribution! @LukasReschke What is the proper way to do this? Create an stable7 and a stable8 branch?

[Henni]

I added $csp->addAllowedScriptDomain('\'self\''); and made the code backwards compatible like @LukasReschke described at https://github.com/owncloud/maps/issues/47#issuecomment-129461851.

[v1r0x]

For me this only works, if I add

$csp->addAllowedImageDomain('*');

[Henni]

@v1r0x What errors are shown with the current implementation?

[v1r0x]

Content Security Policy: Die Einstellungen der Seite haben das Laden einer Ressource auf self blockiert ("script-src https://owncloud.my.url https://owncloud.my.url 'unsafe-eval'").
onsubmit attribute on DIV element

Content Security Policy: Die Einstellungen der Seite haben das Laden einer Ressource auf http://otile3.mqcdn.com/tiles/1.0.0/osm/15/17210/11312.png blockiert ("img-src https://owncloud.my.url https://otile1.mqcdn.com https://otile2.mqcdn.com https://otile3.mqcdn.com https://otile4.mqcdn.com").
Content Security Policy: Die Einstellungen der Seite haben das Laden einer Ressource auf http://otile2.mqcdn.com/tiles/1.0.0/osm/15/17210/11311.png blockiert ("img-src https://owncloud.my.url https://otile1.mqcdn.com https://otile2.mqcdn.com https://otile3.mqcdn.com https://otile4.mqcdn.com").
Content Security Policy: Die Einstellungen der Seite haben das Laden einer Ressource auf http://otile2.mqcdn.com/tiles/1.0.0/osm/15/17209/11312.png blockiert ("img-src https://owncloud.my.url https://otile1.mqcdn.com https://otile2.mqcdn.com https://otile3.mqcdn.com https://otile4.mqcdn.com").

[Henni]

@v1r0x did you use the master branch or did you checkout this pull-request when these errors occurred?

[v1r0x]

I checked out the pull request

[Henni]

@v1r0x which browser did you use?

[brantje]

$csp->addAllowedImageDomain('www.openstreetmap.org');

[DJaeger]

@brantje: The blocked images doesn't came from osm but from mq, so this doesn't work.

The issue seems to be, that the images to load use http and allowed is only https. The csp class seems to prepend the currently used protokoll and @v1r0x seems to use https to access his owncloud. But we have to force http as mq only delivers tiles over http.

[DJaeger]

As of https://developer.mozilla.org/en-US/docs/Web/Security/CSP/CSP_policy_directives we can also allow http://*.mqcdn.com or http://otile1.mqcdn.com http://otile2.mqcdn.com http://otile3.mqcdn.com http://otile4.mqcdn.com

[DJaeger]

As the csp class simply pass through the values this should work.

[Henni]

@DJaeger I applied your fix http://*.mqcdn.com

@v1r0x could you retest please.

[v1r0x]

works fine now! :) But I had to add

$csp->addAllowedImageDomain('https://api.tiles.mapbox.com');

as well for the markers

[DJaeger]

@v1r0x: You are right, they are also requested from another origin. I think they could be added to this repo some time later, as they can be found here: https://github.com/mapbox/maki This would minimize dependencies and requests to other origins and can also be handed by the caching preferences of owncloud.

[Henni]

For now I'll add https://api.tiles.mapbox.com to the Content-Security-Policies.

[Henni]

If you're happy with these changes I'll squash the commits.

[Henni]

I created an issue to integrate the icons: #52

[DJaeger]

Forcing https for the API is great, as it is hardcoded to https. Adding the specific subdomains would be more secure, but I think we can trust a subdomain wildcard for this domain enough to let it as it is. The third code comment is wrong, as it allows scripts to be loaded and not images.

[Henni]

I didn't completely understand the last rule. As far as I understand it was added because some image which is included in the script couldn't load. What would be a better comment in your opinion @DJaeger?

[brantje]

Looks good. :+1:

[jancborchardt]

Nice, looks great! :) @Henni cool to see you in here as well!

@LukasReschke @DJaeger can you give this a review too please? So with this Maps will only be compatible with 8.1+? (I’m fine with that, just confirming.)

[Henni]

@jancborchardt it should still be compatible for < 8.1 maybe @LukasReschke can confirm this.

[DJaeger]

An image can only be in a script as "data:" url, where it would require

$csp->addAllowedImageDomain('data:');

and as I have now seen, we also require that for contact images.
This:

$csp->addAllowedScriptDomain('\'self\'');

is required to load scripts by scripts from the server. I don't know where we do it, but @v1r0x had such a request, that was blocked. I'm not sure, but it could be coursed by wrongly requesting json.

[DJaeger]

@jancborchardt: If I'm right it should be compatible with each oc8 version

[Henni]

@DJaeger what about oC7?

[DJaeger]

I'm sorry, I was wrong. We have not changed any compatibility with this PR.

[jancborchardt]

So we merge? :+1: from me :)

[DJaeger]

No, please add this line before:

$csp->addAllowedImageDomain('data:');

[DJaeger]

And change

// images inside scripts

to

// scripts

[jancborchardt]

@Henni and maybe as a last thing squash the commits into one to keep history clean. :)

[DJaeger]

If these changes are done also :+1: from me

[LukasReschke]

Valid approach. I'd make it easier by just whitelisting * and also change the comment. Besides this: Good work :+1:

[Henni]

I removed $csp->addAllowedScriptDomain('\'self\''); as this is set by default. I added $csp->addAllowedImageDomain('data:'); as it is needed for inline images.

To sum it up, we now allow:

• Map tiles (on four different subdomains) - addAllowedImageDomain('http://*.mqcdn.com');
• Map icons (which will change with #52) - addAllowedImageDomain('https://api.tiles.mapbox.com');
• Inline images (for loading tile and contact thumbnails when fixes) - addAllowedScriptDomain('data:');

If you are all ok with this, let's merge it!

[brantje]

:+1: code looks good

[DJaeger]

:+1: from me.

[Henni]

@brantje @DJaeger great thanks!