1485177 - Expose the |site| on nsIPrincipal

Status: RESOLVED FIXED

(Core :: Security: CAPS, enhancement, P2)

Description

[Nika Layzell [:nika] (ooo, ni? for response)]

This will make it easier in the future to encode certain behaviours which depend on a principal's |site|. The basic restriction of a site is that two documents with different sites may never be Same-Origin-Domain (a.k.a. they will never be considered |subsumesConsideringDomain|).

Comment 1

[Nika Layzell [:nika] (ooo, ni? for response)]

Attachment: Bug 1485177 - Add |siteOrigin| information to nsIPrincipal

Comment 2

[Nika Layzell [:nika] (ooo, ni? for response)]

Attachment: Bug 1485177 - Part 2: ASDHASKJDHASKJDHAS, r=ehsan

Comment 3

[Nika Layzell [:nika] (ooo, ni? for response)]

Attachment: Bug 1485177 - Part 3: BBBBBJDHASKJDHAS, r=ehsan

Depends on D4702

Comment 4

[(no longer active)]

Comment on attachment 9003582 [details]
Bug 1485177 - Add |siteOrigin| information to nsIPrincipal

:Ehsan Akhgari has approved the revision.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/33c7b0ea5caa
Add |siteOrigin| information to nsIPrincipal, r=Ehsan

Comment 6

[Tiberius Oros[:tiberius_oros]]

Backed out changeset 33c7b0ea5caa (bug 1485177) for assertion failure at builds/worker/workspace/build/src/caps/ContentPrincipal.cpp

Push with the backout:
https://hg.mozilla.org/integration/autoland/rev/30c890dba6ea44e4dbea9ef8bf82a67eac67eee2

Push with the failure:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=33c7b0ea5caaa654c72cce43124db1ebc052dead

Failure log file:
https://treeherder.mozilla.org/#/jobs?repo=autoland&fromchange=3050ceae2cbb384fb0535e3c7c2425805a9dc085&selectedJob=196995953

Comment 7

[Bobby Holley (:bholley)]

Would be good to coordinate this with the work in bug 1487032.

Comment 8

[Jan de Mooij [:jandem]]

(In reply to Bobby Holley (:bholley) from comment #7)
> Would be good to coordinate this with the work in bug 1487032.

I think I could use this for bug 1487032, but ContentPrincipal::GetSiteOrigin will be too slow to call on each wrap operation. 

How do people feel about caching that string in BasePrincipal? I suppose I could also call GetSiteOrigin when creating the CompartmentPrivate and then cache it there...

Comment 9

[Jan de Mooij [:jandem]]

Actually for bug 1487032 I think we have to do a subsumes check to support

  Wrap(Compartment-With-ExpandedPrincipal, Compartment-With-ContentPrincipal)

Because here we want to return a transparent wrapper if the expanded principal subsumes the other principal (and an opaque wrapper otherwise)?

bholley suggested returning a new principal representing the site, but I think I can build that on top of GetSiteOrigin added here.

Nika, for your use case we don't have to worry about comparing an expanded principal to a content principal?

Comment 10

[Nika Layzell [:nika] (ooo, ni? for response)]

(In reply to Jan de Mooij [:jandem] from comment #8)
> (In reply to Bobby Holley (:bholley) from comment #7)
> > Would be good to coordinate this with the work in bug 1487032.
> 
> I think I could use this for bug 1487032, but
> ContentPrincipal::GetSiteOrigin will be too slow to call on each wrap
> operation. 

Definitely - We can make it fast if we need though.

> How do people feel about caching that string in BasePrincipal? I suppose I
> could also call GetSiteOrigin when creating the CompartmentPrivate and then
> cache it there...

I'd be fine with caching the value for ContentPrincipal objects, for non-ContentPrincipal objects, we already cache it (as it's just the origin).

I just didn't use it for this case, because I didn't see performance as relevant for the use case I had in mind.

(In reply to Jan de Mooij [:jandem] from comment #9)
> Actually for bug 1487032 I think we have to do a subsumes check to support
> 
>   Wrap(Compartment-With-ExpandedPrincipal, Compartment-With-ContentPrincipal)
> 
> Because here we want to return a transparent wrapper if the expanded
> principal subsumes the other principal (and an opaque wrapper otherwise)?
> 
> bholley suggested returning a new principal representing the site, but I
> think I can build that on top of GetSiteOrigin added here.

Yeah, I think that makes sense as an approach to take.

> Nika, for your use case we don't have to worry about comparing an expanded
> principal to a content principal?

I don't think we'd have to worry about that in my case, as expanded principals don't often show up in the environment which I'd be wanting to use these. 

I think we could make ExpandedPrincipal call a method which performs the comparison with it's inner principals, which could have the desired effect? Not sure.

Comment 11

[Jan de Mooij [:jandem]]

Makes sense. After your patch lands I can just make my changes on top of it I think, it definitely helps :)

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/434d6bf22d1d
Add |siteOrigin| information to nsIPrincipal r=Ehsan

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/434d6bf22d1d