Bug 34296 – Provide a way for WebKit clients to specify a more granular policy for cross-origin frame access

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Mike Thole]

Similar to https://bugs.webkit.org/show_bug.cgi?id=24853, which provides whitelist functionality for XHR access, WebKit should provide a similar method for allow cross-origin frame access.

[Mike Thole]

Created attachment 47724 [details]
Add whitelist check to SecurityOrigin::canAccess()

[Mike Thole]

In related bug 24853, Aaron Boodman mentioned that:
"For example, we don't really need (and probably don't want) the ability to do cross-frame scripting."

I wonder what the rationale for not wanting the ability to do cross-frame scripting was?  I added patch (47724) that adds a whiltelist check to SecuriyOrigin::canAccess().  If we still don't want cross-frame access to go along with XHR access, we'll need to do something a little more involved.

[Adam Barth]

I don't think this patch is a good idea.  In particular, asymmetric access relations are the source of many security vulnerabilities.  For more information, please read:

http://www.adambarth.com/papers/2009/barth-jackson-li.pdf

[Mike Thole]

Currently, a WebKit client that wants to allow cross-origin frame access can do so using WebView's registerURLSchemeAsLocal:, assuming that the allowUniversalAccessFromFileURLs() setting enabled.

What I'm trying to do is allow cross-origin frame access without also treating the custom URL protocol as a local protocol.  The asymmetric access vulnerabilities mentioned in the linked paper are valid reasons for caution, but they can be prevented with careful programming.  WebKit clients should be able to choose to allow cross-frame access without going crazy and giving the URL protocol local access.

The white list functionality added for 24853 seemed ideal for this purpose.  Maybe that API could be modified or added to, such that a WebKit client could explicitly opt-in to cross-origin frame access?  One simple solution would be to add an additional parameter to whiteListAccessFromOrigin() that specified what type of access was whitelisted (just XHR or frame access).

Do you have any suggestions on how this could best be accomplished?

[Maciej Stachowiak]

(In reply to comment #4)
> Currently, a WebKit client that wants to allow cross-origin frame access can do
> so using WebView's registerURLSchemeAsLocal:, assuming that the
> allowUniversalAccessFromFileURLs() setting enabled.
> 
> What I'm trying to do is allow cross-origin frame access without also treating
> the custom URL protocol as a local protocol.  The asymmetric access
> vulnerabilities mentioned in the linked paper are valid reasons for caution,
> but they can be prevented with careful programming.  WebKit clients should be
> able to choose to allow cross-frame access without going crazy and giving the
> URL protocol local access.
> 
> The white list functionality added for 24853 seemed ideal for this purpose. 
> Maybe that API could be modified or added to, such that a WebKit client could
> explicitly opt-in to cross-origin frame access?  One simple solution would be
> to add an additional parameter to whiteListAccessFromOrigin() that specified
> what type of access was whitelisted (just XHR or frame access).
> 
> Do you have any suggestions on how this could best be accomplished?

I can see how clients may have valid use cases for an XHR-only whitelist, given Adam's paper. I can also understand that there are valid use cases for giving cross-frame scripting access without granting local access.

I think your idea of extending the whitelisting mechanism makes sense. That way both sets of use cases can be satisfied.

[Adam Barth]

I really think we should keep canAccess symmetric.  The web platform isn't designed for asymmetric access.  It might appear to work, but, in reality, the "lesser" origin can almost always compromise the "greater" origin.

[Mike Thole]

Although canAccess() isn't entirely symmetric today, given that some SecurityOrigins are granted universal access and canAccess() respects that.

[Adam Barth]

> Although canAccess() isn't entirely symmetric today, given that some
> SecurityOrigins are granted universal access and canAccess() respects that.

Indeed.  I'm been on a multi-year odyssey to remove these cases.  I think this situation is removed in the default settings for every port except the Mac port.  My understanding is that future version of Safari will use a the non-default setting but that we're worried about non-Safari clients of WebKit relying on the old (insecure) behavior.

[Mike Thole]

Created attachment 49934 [details]
New patch.  Adds SecurityOrigin::allowDOMAccessFromOrigin() for explicitly granting cross-origin DOM access via a whitelist

I think it would be more useful to discuss the specifics of a patch, rather than generalities.  I've created a full patch that renames SecurityOrigin::whiteListAccessFromOrigin() to SecurityOrigin::allowNetworkAccessFromOrigin() and adds SecurityOrigin::allowDOMAccessFromOrigin().  This provides the SPI needed by some clients to explicitly choose to whitelist cross-origin DOM access.

[WebKit Review Bot]

Attachment 49934 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/329033

[Darin Adler]

Comment on attachment 49934 [details]
New patch.  Adds SecurityOrigin::allowDOMAccessFromOrigin() for explicitly granting cross-origin DOM access via a whitelist

Chromium build failure:

src/WebSecurityPolicy.cpp: In static member function ‘static void WebKit::WebSecurityPolicy::whiteListAccessFromOrigin(const WebKit::WebURL&, const WebKit::WebString&, const WebKit::WebString&, bool)’:
src/WebSecurityPolicy.cpp:64: error: ‘whiteListAccessFromOrigin’ is not a member of ‘WebCore::SecurityOrigin’
src/WebSecurityPolicy.cpp: In static member function ‘static void WebKit::WebSecurityPolicy::resetOriginAccessWhiteLists()’:
src/WebSecurityPolicy.cpp:71: error: ‘resetOriginAccessWhiteLists’ is not a member of ‘WebCore::SecurityOrigin’

[Adam Barth]

Maciej makes the point that we already have a number of dangerous APIs that we expose to the client.  It might make sense to have a large comment warning clients that this API is dangerous in subtle ways.

In any case, I feel like I've said my piece.  If another reviewer thinks the benefits of having this API outweigh the risks to unsuspecting clients, I'm not going to stand in the way.

[Mike Thole]

Created attachment 50042 [details]
Updated patch: Updates chromium's WebSecurityPolicy

[Mike Thole]

With Adam and Maciej's help I found a different way to achieve what I needed without adding this API.  Closing this bug.