1125627 - Allow XUL images to indicate an origin/principal to use for their load other than their document's origin/principal

Status: NEW

(Core :: XUL, defect)

Description

[:Gijs (he/him)]

Because XUL is no longer enabled in non-privileged content, XUL images almost by definition appear in privileged content.

When we load images in them, for instance in XUL notification items or for favicon images, it's often desirable to apply security properties from a different document to the load (namely the content document that specified the favicon or notification).

It's currently not easy to do this. The simplest would probably be if we could indicate a principal/origin via an attribute on the image element, but perhaps we need something more robust?

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I think what we need is the ability to specify the loadingNode that should be used for the image load. This could then be set to the Document node of some content page. I don't know of situations where we would need to set a loadingPrincipal or a triggeringPrincipal.

However I couldn't find where in Gecko we kick off <xul:image> loads. So I don't have an implementation strategy to recommend.

Also, e10s is going to make this more awkward since the loadingNode lives in the child process, whereas the <xul:image> generally will live in the parent...

Comment 2

[:Gijs (he/him)]

(In reply to Jonas Sicking (:sicking) from comment #1)
> I think what we need is the ability to specify the loadingNode that should
> be used for the image load. This could then be set to the Document node of
> some content page. I don't know of situations where we would need to set a
> loadingPrincipal or a triggeringPrincipal.

This doesn't really easily work with the DOM, nor with e10s, though, in that you can't declaratively associate the one with the other as an attribute, nor can you even refer to nodes in the content process in e10s... That's why I suggested an attribute with a principal...

Is there any reason not to just use the principal? (which we *could* use an attribute for, and which we can read from the browser node even in e10s)
I'm guessing we wouldn't know CSP directives... is that the only thing?

(the attribute thing becomes more important because XBL and doing lots of document.getAnonymousElementByAttribute to get to all these images from script is going to be a pain)

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

By "attribute", do you mean an attribute in the markup (<elem x="y">, and element.setAttribute("x", "y")) or do you mean a JS property (element.x = y)?

I don't think that we can set a markup attribute to a principal. At least not if we want to retain things like CSP policies.

But we can set a JS property to a principal object.

And if it wasn't for e10s we could set a JS property to a DOM Node as well I would think. But I think you are right that that's probably never useful any more given e10s.

So yeah, I think the solution is to add something like

xulImageElement.loadingPrincipal = principalObject;

Which would have to be set before the .src is set. Does that match your thinking?

Comment 4

[:Gijs (he/him)]

(In reply to Jonas Sicking (:sicking) from comment #3)
> By "attribute", do you mean an attribute in the markup (<elem x="y">, and
> element.setAttribute("x", "y")) or do you mean a JS property (element.x = y)?
> 
> I don't think that we can set a markup attribute to a principal. At least
> not if we want to retain things like CSP policies.

Ah, I assumed that principals are basically URIs (which obviously can go in DOM attributes). If it's more complex than that, then yeah, it doesn't fit in an attribute.

> But we can set a JS property to a principal object.
> 
> And if it wasn't for e10s we could set a JS property to a DOM Node as well I
> would think. But I think you are right that that's probably never useful any
> more given e10s.
> 
> So yeah, I think the solution is to add something like
> 
> xulImageElement.loadingPrincipal = principalObject;
> 
> Which would have to be set before the .src is set. Does that match your
> thinking?

Yes, we'd need to update the current message passing for onLinkAdded and onLinkChanged and so on to pass along the principal, and then do something with it... although it seems we already do quite some verification here, it probably makes sense to see if we can adequately replace that by just passing the principal and letting that take care of it. :-)

http://mxr.mozilla.org/mozilla-central/source/browser/modules/ContentLinkHandler.jsm#135

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

passing a principal through onLinkAdded/Changed sounds doable. The messagemanager already supports sending a principal along with its messages, so if that's what we're using, then this should even be doable in JS.

The idea is definitely to replace existing security checks with simply passing the appropriate principals and security parameters when doing the load. We're not there yet though. For now we're just adding the additional information to all callsites. Next step is to add security checks inside necko. Third step is to remove other security checks.