Closed Bug 968335 Opened 7 years ago Closed 7 years ago

Expose caller principal to JS-implemented webidl

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: mt, Assigned: bholley)

References

(Blocks 2 open bugs, )

Details

Attachments

(7 files, 2 obsolete files)

5.00 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
2.84 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
2.49 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
4.10 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
10.44 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
4.15 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
4.69 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
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().
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #8370985 - Attachment is obsolete: true
Taking over from here.
Assignee: bzbarsky → bobbyholley
I've got all the code done for this - I just need to write tests.
Still sorting out some weirdness with the build changes for the tests for bug 923904, but we can start getting this reviewed.
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.
Attachment #8374948 - Flags: review?(bzbarsky)
This will let us ask whether the AutoCxPusher is stack-top.
Attachment #8374949 - Flags: review?(bzbarsky)
This will allow us to downcast from a stack entry to an AutoEntryGlobal, and
thereby get at the AutoCxPusher.
Attachment #8374950 - Flags: review?(bzbarsky)
Attachment #8371028 - Attachment is obsolete: true
Attachment #8374952 - Flags: review?(bzbarsky)
Intentionally not flagging these for review until we get the build issues
sorted.
Hopefully sorted out the packaging issues: https://tbpl.mozilla.org/?tree=Try&rev=9480abb18974
Comment on attachment 8374954 [details] [diff] [review]
Part 7 - Tests. v1

All green now. Flagging bz.
Attachment #8374954 - Flags: review?(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
Attachment #8374947 - Flags: review?(bzbarsky) → review+
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
Attachment #8374948 - Flags: review?(bzbarsky) → review+
Comment on attachment 8374949 [details] [diff] [review]
Part 3 - Use an AutoCxPusher directly in Auto{Entry,Incumbent}Global. v1

r=me
Attachment #8374949 - Flags: review?(bzbarsky) → review+
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?
Attachment #8374950 - Flags: review?(bzbarsky) → review+
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.
Attachment #8374951 - Flags: review?(bzbarsky) → review+
Comment on attachment 8374952 [details] [diff] [review]
Part 6 - Implement Cu.getWebIDLCallerPrincipal. v1

r=me
Attachment #8374952 - Flags: review?(bzbarsky) → review+
Comment on attachment 8374954 [details] [diff] [review]
Part 7 - Tests. v1

r=me
Attachment #8374954 - Flags: review?(bzbarsky) → review+
(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.
Yes, and wrap() is slow....  I guess we're doing a bunch of it anyway here.  :(
Blocks: 994935
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.