968335 - Expose caller principal to JS-implemented webidl

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Martin Thomson [:mt:]]

JS-implemented webidl is unable to accurately (i.e., securely) determine the principal of the content object that invoked it.  Using window.document.nodePrincipal exposes the JS to the chance that navigation could occur, causing this to be spoofed.

In C++, this would involve calling GetSubjectPrincipal().

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: This gives us a caller compartment to work with in the CallSetup constructor.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Codegen bits that even compile

Comment 3

[Bobby Holley (:bholley)]

Taking over from here.

Comment 4

[Bobby Holley (:bholley)]

I've got all the code done for this - I just need to write tests.

Comment 5

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=590b4f0d0bff

Comment 6

[Bobby Holley (:bholley)]

Still sorting out some weirdness with the build changes for the tests for bug 923904, but we can start getting this reviewed.

Comment 7

[Bobby Holley (:bholley)]

Attachment: Part 1 - Add accessors to the script settings stack entries themselves, not just the globals. v1

Comment 8

[Bobby Holley (:bholley)]

Attachment: Part 2 - Add an API to determine if a given AutoCxPusher corresponds to the stack-top cx push. v1

This patch, and those following, are part of an epic quest to make this API
work properly despite the fact that we don't yet have GetEntryGlobal. See
the comment a few patches forward.

Comment 9

[Bobby Holley (:bholley)]

Attachment: Part 3 - Use an AutoCxPusher directly in Auto{Entry,Incumbent}Global. v1

This will let us ask whether the AutoCxPusher is stack-top.

Comment 10

[Bobby Holley (:bholley)]

Attachment: Part 4 - Make Auto{Entry,Incumbent}Global inherit ScriptSettingsStackEntry. v1

This will allow us to downcast from a stack entry to an AutoEntryGlobal, and
thereby get at the AutoCxPusher.

Comment 11

[Bobby Holley (:bholley)]

Attachment: Part 5 - Implement GetCallerPrincipalOverride. v1

Comment 12

[Bobby Holley (:bholley)]

Attachment: Part 6 - Implement Cu.getWebIDLCallerPrincipal. v1

Comment 13

[Bobby Holley (:bholley)]

Attachment: Part 7 - Tests. v1

Intentionally not flagging these for review until we get the build issues
sorted.

Comment 14

[Bobby Holley (:bholley)]

Hopefully sorted out the packaging issues: https://tbpl.mozilla.org/?tree=Try&rev=9480abb18974

Comment 15

[Bobby Holley (:bholley)]

Comment on attachment 8374954 [details] [diff] [review]
Part 7 - Tests. v1

All green now. Flagging bz.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8374947 [details] [diff] [review]
Part 1 - Add accessors to the script settings stack entries themselves, not just the globals. v1

r=me

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8374948 [details] [diff] [review]
Part 2 - Add an API to determine if a given AutoCxPusher corresponds to the stack-top cx push. v1

r=me

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8374949 [details] [diff] [review]
Part 3 - Use an AutoCxPusher directly in Auto{Entry,Incumbent}Global. v1

r=me

Comment 19

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8374950 [details] [diff] [review]
Part 4 - Make Auto{Entry,Incumbent}Global inherit ScriptSettingsStackEntry. v1

Do we want to mark these as stack classes while we're here?

Comment 20

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8374951 [details] [diff] [review]
Part 5 - Implement GetCallerPrincipalOverride. v1

How cheap is GetSubjectPrincipal?  We're going to do it on every JS-implemented WebIDL call/get/set, right?

>+            callSetup += str(bool(isJSImplementedDescriptor(self.descriptorProvider))).lower()

  callSetup += toStringBool(isJSImplementedDescriptor(self.descriptorProvider))

r=me modulo that performance concern.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8374952 [details] [diff] [review]
Part 6 - Implement Cu.getWebIDLCallerPrincipal. v1

r=me

Comment 22

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8374954 [details] [diff] [review]
Part 7 - Tests. v1

r=me

Comment 23

[Bobby Holley (:bholley)]

(In reply to Boris Zbarsky [:bz] from comment #19)
> Do we want to mark these as stack classes while we're here?

We could, but we may need to undo that again with bug 971673. I'd rather sort that out first.

(In reply to Boris Zbarsky [:bz] from comment #20)
> How cheap is GetSubjectPrincipal?  We're going to do it on every
> JS-implemented WebIDL call/get/set, right?

Pretty cheap. It boils down to:

nsJSPrincipals::get(GetCompartmentPrincipals(GetContextCompartment(XPCJSRuntime:Get()->GetJSContextStack()->Peek())))

So basically just pointer chasing. We do this several times for every wrap() call, for comparison.

Comment 24

[Boris Zbarsky [:bzbarsky]]

Yes, and wrap() is slow....  I guess we're doing a bunch of it anyway here.  :(

Comment 25

[Bobby Holley (:bholley)]

Green try push in comment 14.

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/6a0ebe4ddd01
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/2a530ceeac5d
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/b057716623f9
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/cac7844c804d
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/5360c2573b11
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/4503fdf32a31
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/4fc776ee6852

Comment 26

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/1e8c2e85575a for b2g emulator bustage a la https://tbpl.mozilla.org/php/getParsedLog.php?id=34717947&tree=Mozilla-Inbound

Comment 27

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=d607ce9c51e6

Comment 28

[Bobby Holley (:bholley)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/ccf60cc8004b
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/b4c42334f112
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/feacaec8b463
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/4e21b5c857cb
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/95570aef1a27
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/c7c4abfa527d
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/032861ed6fcc

Comment 29

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/ccf60cc8004b
https://hg.mozilla.org/mozilla-central/rev/b4c42334f112
https://hg.mozilla.org/mozilla-central/rev/feacaec8b463
https://hg.mozilla.org/mozilla-central/rev/4e21b5c857cb
https://hg.mozilla.org/mozilla-central/rev/95570aef1a27
https://hg.mozilla.org/mozilla-central/rev/c7c4abfa527d
https://hg.mozilla.org/mozilla-central/rev/032861ed6fcc