550442 - (CSP) Handling of object subrequests makes it hard to define policies

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Wladimir Palant]

I enabled CSP on adblockplus.org. However, I noticed that allowing a YouTube video (that I have on the main page) is far from trivial. Specifying object-src www.youtube.com is not sufficient - this video makes subrequests to ytimg.com which have to be allowed as well. What's worse, I have to allow ytimg.com in the "catch-all" rule where I wanted to have 'self' and nothing else.

I see two solutions:

1) Always allow object subrequests. After all, these are made by the object that the webmaster typically doesn't control. It's not obvious where ytimg.com requests came from. Then again, same is true for third-party scripts that make additional requests - and restricting those is still desirable.

"Patch" for contentSecurityPolicy.js:

-  csp._MAPPINGS[cp.TYPE_OBJECT_SUBREQUEST] = cspr_sd.ALLOW;

2) Make object subrequests respect object-src instead of going to catch-all immediately. After all, these requests belong to the object previously allowed, this also gives the webmaster more granular control.

"Patch" for contentSecurityPolicy.js:

-  csp._MAPPINGS[cp.TYPE_OBJECT_SUBREQUEST] = cspr_sd.ALLOW;
+  csp._MAPPINGS[cp.TYPE_OBJECT_SUBREQUEST] = cspr_sd.OBJECT_SRC;

Comment 1

[Sid Stamm [:geekboy or :sstamm]]

Attachment: fix

I can totally see this problem, thanks for the feedback.  I'm trying to remember if there was a reason we specifically put TYPE_OBJECT_SUBREQUEST in the "ALLOW" bucket, but I'm not coming up with any really good reasons.  I think maybe it was an effort to minimize subordinate loads from flash, but it seems to be pretty common.

> -  csp._MAPPINGS[cp.TYPE_OBJECT_SUBREQUEST] = cspr_sd.ALLOW;
> +  csp._MAPPINGS[cp.TYPE_OBJECT_SUBREQUEST] = cspr_sd.OBJECT_SRC;

I think this looks like a good fix.  Actual patch attached.

Comment 2

[Daniel Veditz [:dveditz]]

(In reply to comment #1)
> I'm trying to remember if there was a reason we specifically put
> TYPE_OBJECT_SUBREQUEST in the "ALLOW" bucket, but I'm not coming up with
> any really good reasons.

I think it's a bug, we talked about this case and I thought we agreed subordinate loads from an object is subject to the object policy for consistency of behavior. Otherwise you can get the same kind of end-run around the policy you can with redirects.

Comment 3

[Daniel Veditz [:dveditz]]

Comment on attachment 430644 [details] [diff] [review]
fix

r=dveditz

Comment 4

[Daniel Veditz [:dveditz]]

http://hg.mozilla.org/mozilla-central/rev/8997036f3497