Bug 104480

Summary: CSP: XHR from an isolated world should bypass a page's policy.
Product: WebKit Reporter: Mike West <mkwst>
Component: WebCore Misc.Assignee: Mike West <mkwst>
Severity: Normal CC: abarth, akapuya, dbates, eric, gyuyoung.kim, japhet, linshunghuang, mkwst+watchlist, rakuco, scripts, webkit.review.bot, yutak
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 85558    
Description Flags
Patch none

Description Mike West 2012-12-09 04:33:04 PST
1. Yay! Facebook is using CSP!
2. Oh noes! Facebook extensions are breaking!

See: http://stackoverflow.com/questions/13786918/refused-to-connect-to-url-because-it-violates-the-following-content-security-p for some detail.

In a nutshell, we should do the same check for `connect-src` that we're currently doing for CachedResourceLoader.
Comment 1 Mike West 2012-12-10 03:03:17 PST
Created attachment 178501 [details]
Comment 2 Mike West 2012-12-10 03:06:25 PST
Hey Eric! Are you a good person to review this patch in Adam's stead?
Comment 3 Eric Seidel (no email) 2012-12-10 11:45:11 PST
Sadly, no.  This really should wait for Adam's return.  Just add a note to your calendar to email him Jan 3rd. :)
Comment 4 Eric Seidel (no email) 2012-12-10 11:46:09 PST
If we absolutely need a fix before then, I can review it, but my understanding of the CSP security model is not nearly as deep as Adam's.
Comment 5 Mike West 2012-12-10 12:28:22 PST
(In reply to comment #4)
> If we absolutely need a fix before then, I can review it, but my understanding of the CSP security model is not nearly as deep as Adam's.

Hrm. We don't absolutely need anything. :) This just jumped onto my radar now that Facebook launched CSP support and we're getting more bug reports.

This patch doesn't do anything really new, it simply applies the check we added to CachedResourceLoader to the areas covered by `connect-src` (WebSockers, XHR, and EventSource). I was hoping it'd be an easy patch to review, but I agree with you completely that we don't want to make mistakes in this area.

*shrug* I'll get in line for Adam's return. He really is the right guy to review.
Comment 6 Build Bot 2012-12-11 13:01:45 PST
Comment on attachment 178501 [details]

Attachment 178501 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15257797

New failing tests:
Comment 7 Avi K 2012-12-13 03:21:27 PST
Thanks guys for handling this, this is really a major issue for many extensions
Comment 8 Oliver Schlöbe 2012-12-13 05:35:08 PST
Thanks also! I am the topic starter of the topic Mike West referenced in his initial post, and am grateful he went and created this bug report a few minutes later. :) Thanks guys!
Comment 9 Mike West 2013-01-04 00:19:30 PST
Created attachment 181279 [details]
Comment 10 Adam Barth 2013-01-04 09:53:57 PST
Comment on attachment 181279 [details]

Comment 11 WebKit Review Bot 2013-01-04 11:10:31 PST
Comment on attachment 181279 [details]

Clearing flags on attachment: 181279

Committed r138817: <http://trac.webkit.org/changeset/138817>
Comment 12 WebKit Review Bot 2013-01-04 11:10:37 PST
All reviewed patches have been landed.  Closing bug.