Page MenuHomePhabricator

Add '*wmflabs.org' to the Content Security Policy of Slimapp
Closed, DeclinedPublic

Description

This is about wikimedia/slimapp.

Although AbstractApp::configureHeaderMiddleware() can easily be overridden by a prooject using it, I'd like to suggest a small modification to the Content Security Policy header as it's defined in Slimapp:

This would make it possible to use files from e.g. //tools-static.wmflabs.org/cdnjs

The change is just to add *.wmflabs.org to some items in the Content-Security-Policy header:

default-src 'self' *.wmflabs.org; 
style-src 'self' 'unsafe-inline' *.wmflabs.org;

I'm not sure what other ramifications this might have though, so I thought I'd ask here. :-)

Event Timeline

Trusting the whole of labs is terrible for security. Is this for something running on a wikimedia.org domain? If so I doubt you'll be able to trust any of labs really.

Hm, good point! Okay, how about changing it to tools-static.wmflabs.org/cdnjs ?

This is a framework for building tools with, so pretty much all of them I'd imagine would be using something from that CDN.

For example, CopyPatrol is currently overriding the framework's default to make it default-src 'self' *; etc. which is even less secure than trusting all of Labs.

Poking @bd808 to look into this when he's back to work. Since Slimapp it's primarily his app, he should decide the fate of this task.

Aside, @Samwilson It is not encouraged to create tasks without any associated projects which risks losing them in the Phabricator abyss. If unsure, add Community-Tech and we can help find out the best project tags for it. Although for this task I can't think of any relevant tags.

For example, CopyPatrol is currently overriding the framework's default to make it default-src 'self' *; etc. which is even less secure than trusting all of Labs.

I agree that's horrible. When we started with the app, we were loading bootstrap from external libraries (not the tool labs versions) and I resorted to allowing everything to avoid hiccups during testing. This should be fixed, thanks for reminding me!

Thanks @Niharika :) Yes, I wasn't sure about protocol for this; I just figured I'd note it down and at the very least it'd be on my own open-task list.

Hm, good point!Okay, how about changing it to tools-static.wmflabs.org/cdnjs ?

I think you'd need a / on the end of that, otherwise its just cdnjs, not subdirectories. Also should consider specifying https: only.

More importantly, i recently read that angularjs can be used to bypass csp restrictions, thus this is not safe so long as angular is in the allowed sources. See https://research.google.com/pubs/pub45542.html

I don't like the idea of making the base slimapp framework allow tools or tools-static/cdnjs by default. This framework is used in one production application today (Wikimedia Scholarships) and should be used in more (Grant review). It's also not exclusively meant for building tools or even Wikimedia applications.

We could do something to make it a lot easier to configure however. Striker is using a Django CSP library that is easily configurable. It would be pretty easy to follow a similar configuration pattern in slimapp.

Ah, yes I guess I was being a bit narrow in my view of it just being for tools or other Wikimedia apps. In that case, it's really pretty simple to override AbstractApp::configureHeaderMiddleware() in one's own application, so maybe it's more a matter of adding something to the documentation. Then, of course, only the precise applicable src URLs would be added.