524771 - XPCSafeJSObjectWrapper needs to be able to wrap objects for access from different principals

Status: RESOLVED INACTIVE

(Core :: XPConnect, defect)

Description

[Atul Varma [:atul]]

For Jetpack, we need to allow non-chrome-privileged Jetpacks to be able to safely interact with code on other pages.  After talking to mrbkap, one easy way to allow access at the XPCSafeJSObjectWrapper-level is to modify its constructor to take a second optional parameter, which is the nsIPrincipal of the client that the wrapper is to be given to.  It's obviously important that non-chrome-privileged code be denied access to this functionality, but this should be enforced by default, since only chrome code has access to nsIPrincipal objects.

Comment 1

[Atul Varma [:atul]]

I played around a bit with making a patch for this bug recently, and ran into some difficulties.  This is what I've got so far, very much a WIP:

  http://hg.mozilla.org/users/avarma_mozilla.com/cow-patches/file/aa2030bcef32/sjow.patch

As you can see, I'm doing a lot of "arm-twisting" that's almost certainly negatively affecting the security of the platform.  Just putting a passed-in nsIPrincipal into the principal slot in the SJOW constructor doesn't seem to do the trick, as CallerCanAccess() is still called by all wrapper functions, and it only looks at the principals of the calling context and the unsafe object that the SJOW wraps--which means that wrapping e.g. a SJOW w/ a "google.com" principal still can't be accessed by a jetpack with a principal of "foo.com", since all the CallerCanAccess() checks fail.  So I had to change CallerCanAccess() to take the principal slot's principal into account, which felt strange.

But even then, once all the SJOW's JSClass methods work, ultimately the SafeCallGuard is just changing the principal of the current context to foo.com before accessing an unsafe object associated with google.com, which means that the call won't work.

For reference, the test I'm trying to succeed at is getting a Jetpack at "foo.com" to successfully read the SJOW'd "window.location.href" property of a google.com window (yeah, I know this is really something that should be done with a XPCNativeWrapper, but it's just for example's sake).  Right now the chrome code that negotiates the security model is creating an XPCNativeWrapper for google.com's window object, but passing-in as a second parameter a nsIPrincipal for foo.com.  Accessing window.location.href works from the Jetpack up until we look up the "href" attribute of "location", at which point I think the code is past the SJOW wrapping and throws an exception because foo.com is trying to access an object belonging to google.com.

I suspect I may be severely misinterpreting the changes I need to make to SJOW here.

BTW, Blake, thanks a lot for refactoring these files in revision 5c503628b3ed--they're much easier to read and understand now!

The more I understand how everything works, though, the more it seems like what we really want is a new sort of principal, rather than modifications to wrappers: e.g., an "aggregate principal" that is just a composition of other principals, e.g. "foo.com", "google.com", that subsumes anything that any one of its members subsumes.  That way a Jetpack's principal can just be the composition of all the domains it's been granted access to.  But maybe principals delves into caps and that scary world of the script security manager that we don't want to touch...

Comment 2

[Atul Varma [:atul]]

Just talked to jst, and I'm going to try creating a new principal for Jetpacks to see if it's a tenable solution.

Comment 3

[Atul Varma [:atul]]

As a proof of concept, I wrote a 'dual principal' as a patch to trunk:

  http://hg.mozilla.org/users/avarma_mozilla.com/cow-patches/file/f5fc17c4c297/new-principal.patch

This principal has the concept of a 'primary' and 'secondary' principal, which are given to it at initialization time. The primary principal is delegated-to for nsIPrincipal methods like getURI() and others that don't make sense when dealing with aggregate data, while subsumes() and a few others first delegate to the primary principal, and then delegate to the secondary principal if that didn't work.

Testing this code worked in the case of DOM localStorage and XMLHttpRequests: that is, DOM localStorage was keyed to the primary principal's domain, while XHRs could be made to either the domain of the primary or secondary principals.

The next step here would be to extend this proof-of-concept such that the secondary principal is replaced with a list of principals.  We might also want to make, as jst mentioned to me in passing, an "inverse of system" principal that basically subsumes any principal except the system principal--or perhaps a "subsumes anything with file:, http:, or https: scheme" principal.

Comment 4

[Atul Varma [:atul]]

It should be noted, though, that another alternative to all this principal business is simply to create a "wrapper" in plain chrome JavaScript that is wrapped with a ChromeObjectWrapper and given to untrusted code.  In object-capability terms, this wrapper would be responsible for attenuating access to the object being wrapped--i.e., applying domain checks and whatnot.

The advantages to this approach are:

  * It allows us to attenuate objects in whatever way we choose; for instance, it'd be possible to give client Jetpacks access only to a specific subtree of a page's DOM, or read-only DOM access.  AFAIK, this can't be done with principals and the xpconnect wrappers we have on-hand.

  * It allows us to easily add other developer-ergonomic features that we wouldn't get through standard wrappers: full stack tracebacks from callbacks that raise exceptions, for instance, and automatic de-registration of event handlers when a Jetpack is unloaded.

  * It doesn't require a binary component, so it'd actually be possible to distribute the Jetpack extension as a pure-JS extension for Firefox 3.6.

The disadvantages I see are:

  * It requires us to manually make wrappers for anything we currently get for "free" from XPCSafeJSObjectWrapper or XPCNativeWrapper.  For instance, the entire DOM would need to be wrapped (see caja's "domita" implementation for an idea of how much code this could involve).

  * Because the wrappers are implemented in pure JS, it'd probably make them less computationally efficient than just creating a new principal.

  * Because of the amount of code involved in creating the wrappers, they could be a little less memory efficient than just creating a new principal.

Comment 5

[Damon Sicore (:damons)]

Atul, do you own this?  Still assigned to nobody.  Want to make sure we've all required hands on Jetpack blockers.

Comment 6

[Atul Varma [:atul]]

Nope, we're currently pursuing the option proposed in comment 4. If that proves to be too slow, we'll likely just introduce whatever infrastructure is necessary to make it faster.

Comment 7

[Michael Kohler [:mkohler]]

This is a mass change. Every comment has "assigned-to-new" in it.

I didn't look through the bugs, so I'm sorry if I change a bug which shouldn't be changed. But I guess these bugs are just bugs that were once assigned and people forgot to change the Status back when unassigning.

Comment 8

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.