Bug 73359

Summary: Move postMessage origin check to SecurityOrigin
Product: WebKit Reporter: Karl Koscher <supersat>
Component: UI EventsAssignee: Karl Koscher <supersat>
Status: RESOLVED WONTFIX    
Severity: Normal CC: abarth, creis, dslomov, fishd, levin, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 73337    
Attachments:
Description Flags
Patch
none
Updated patch abarth: review-

Karl Koscher
Reported 2011-11-29 13:46:24 PST
As explained in bug 73337, we need the ability to create frames that accept postMessages intended for any origin. Moving the postMessage origin check from DOMWindow to SecurityOrigin lets us specify that a document has a special SecurityOrigin with the privilege to do so. We also need to call this check from Chromium before we dispatch a postMessage to a WebFrame.
Attachments
Patch (8.38 KB, patch)
2011-11-29 16:05 PST, Karl Koscher
no flags
Updated patch (7.93 KB, patch)
2011-12-01 14:23 PST, Karl Koscher
abarth: review-
Karl Koscher
Comment 1 2011-11-29 16:05:18 PST
WebKit Review Bot
Comment 2 2011-11-29 16:07:05 PST
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Darin Fisher (:fishd, Google)
Comment 3 2011-11-29 21:39:46 PST
+abarth, who should review all SecurityOrigin changes.
Sam Weinig
Comment 4 2011-11-30 00:05:53 PST
Comment on attachment 117065 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117065&action=review > Source/WebCore/page/DOMWindow.cpp:913 > // Check target origin now since the target document may have changed since the simer was scheduled. Might want to fix the typo, simer -> timer, while you are here.
Adam Barth
Comment 5 2011-11-30 01:04:44 PST
Comment on attachment 117065 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117065&action=review > Source/WebCore/page/SecurityOrigin.h:103 > + bool canReceivePostMessage(const SecurityOrigin* intendedOrigin); Maybe canReceivePostMessage => canReceiveMessagesFor ? You want to be clear that it's "for" the intendedOrigin rather than "from" a source origin, even at the call sites. > Source/WebCore/page/SecurityOrigin.h:113 > + // Explicitly grant the ability to receive postMessages from any origin. > + void grantReceivePostMessagesFromAnyOrigin(); Can you explain when we'd want to grant this privilege?
Karl Koscher
Comment 6 2011-12-01 14:23:07 PST
Created attachment 117489 [details] Updated patch (In reply to comment #5) > (From update of attachment 117065 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=117065&action=review > > > Source/WebCore/page/SecurityOrigin.h:103 > > + bool canReceivePostMessage(const SecurityOrigin* intendedOrigin); > > Maybe canReceivePostMessage => canReceiveMessagesFor ? You want to be clear that it's "for" the intendedOrigin rather than "from" a source origin, even at the call sites. Yeah. I was confused a bit myself, but cleaned that all up. > > Source/WebCore/page/SecurityOrigin.h:113 > > + // Explicitly grant the ability to receive postMessages from any origin. > > + void grantReceivePostMessagesFromAnyOrigin(); > > Can you explain when we'd want to grant this privilege? I'm working on getting cross-process postMessages to work. The way we're doing it is by installing a native event listener from the chromium side on a blank, swapped-out document. However, since this document doesn't have an origin, it needs to be able to receive messages intended for any origin. We then call canReceiveMessagesFor from the chromium side right before we dispatch the message. This prevents a race where the origin could change between the time it was checked and the time it was dispatched (which is why we can't simply set the proxy document's origin).
Adam Barth
Comment 7 2011-12-01 14:45:20 PST
Hum... That doesn't sound like the most elegant design. Why not have postMessage ask the embedder if it would like to deliver the message instead? The embedder would then signal that it will deliver the message before we run the security check.
Karl Koscher
Comment 8 2011-12-01 15:34:20 PST
(In reply to comment #7) > Hum... That doesn't sound like the most elegant design. Why not have postMessage ask the embedder if it would like to deliver the message instead? The embedder would then signal that it will deliver the message before we run the security check. This was Darin's suggestion, but I agree that it doesn't seem too elegant. I'll look into this approach.
Karl Koscher
Comment 9 2011-12-01 17:19:53 PST
(In reply to comment #7) > Hum... That doesn't sound like the most elegant design. Why not have postMessage ask the embedder if it would like to deliver the message instead? The embedder would then signal that it will deliver the message before we run the security check. Do you have any opinions on where this interface should be added? WebFrameClient seems like a logical choice, but then we'd need to thread it through FrameLoader and FrameLoaderClient. I'm not sure how much sense that would make. WebViewClient is another option, but it would also needed to be threaded through some unexpected classes (Page, Chrome, ChromeClient). As an alternative, we could add a new client to Page, but that seems like overkill. Thoughts?
Adam Barth
Comment 10 2011-12-01 17:28:26 PST
WebFrameClient is the correct place. There are some layers to drill through, but it shouldn't be too bad.
Karl Koscher
Comment 11 2011-12-05 17:40:04 PST
We're now going to ask the embedder if it wants to deliver the postMessage, making this bug obsolete.
Note You need to log in before you can comment on or make changes to this bug.