Open Bug 1091883 Opened 10 years ago Updated 2 years ago

Investigate if the same origin check in SetReferrer should use triggeringPrincipal instead of loadingPrincipal, and fix if necessary

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
REOPENED
Milestone:
mozilla39
Tracking Flags:
Tracking Status
firefox39 --- fixed

People

(Reporter: geekboy, Unassigned, NeedInfo)

References

(
URL
)

Details

Attachments

(1 file, 3 obsolete files)

Test cases
(deleted), patch
averstak
: review+
Details | Diff | Splinter Review
Once bug 704320 and bug 1083422 land, we need to figure out what the right principal to use for request-same-origin checks in HttpBaseChannel::SetReferrerWithPolicy(). Do we care about the loading principal or the triggering principal? For example, bz suggests this scenario: > Concretely, say you have two pages: page A at http://a.foo.com and page B at > http://b.foo.com. Now if A triggers a location set on a subframe on B, say > to http://b.foo.com/subframe, the referrer will be http://a.foo.com. The > question is whether this is a cross-origin request or not. What is the > request origin in this situation per the spec? https://bugzilla.mozilla.org/show_bug.cgi?id=704320#c220
Depends on: 704320, 1083422
I can help with that.
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Thanks. Draft spec says (may need to be fixed, but here's the current text): > The Origin When Cross-Origin policy specifies that a full URL, > stripped for use as a referrer, is sent as referrer information > when making same-origin requests from a particular global > environment, and only the ASCII serialization of the origin of > the global environment from which a request is made is sent as > referrer information when making cross-origin requests from a > particular global environment. https://w3c.github.io/webappsec/specs/referrer-policy/#referrer-policy-state-origin-when-cross-origin
Attached patch Added test (obsolete) (deleted) — — Splinter Review
Fixed in bug 1113438. Adding a test case here.
Attachment #8542842 - Flags: review?(mcmanus)
Comment on attachment 8542842 [details] [diff] [review] Added test Review of attachment 8542842 [details] [diff] [review]: ----------------------------------------------------------------- lgtm, but I'll delegate referrer test coverage to sid
Attachment #8542842 - Flags: review?(mcmanus) → review?(sstamm)
Comment on attachment 8542842 [details] [diff] [review] Added test Review of attachment 8542842 [details] [diff] [review]: ----------------------------------------------------------------- Yeah, looks good to me too. Covers the case we need.
Attachment #8542842 - Flags: review?(sstamm) → review+
Comment on attachment 8542842 [details] [diff] [review] Added test Review of attachment 8542842 [details] [diff] [review]: ----------------------------------------------------------------- But I think we need another one. This test covers what we needed for bug 1113438, but there's another test at hand. Tanvi brought to my attention the original issue in comment 0. What if content in origin A causes navigation in a subframe of another document that has origin B? Is the parent of the subframe (B) the origin to compare to see if the referrer is cross-origin or is the triggering code (A) the one to consider? So we need another test.
Attached patch Test case for comment #0 (obsolete) (deleted) — — Splinter Review
Yep, updated the test. It uses the page that triggered the navigation for both the referrer header and the origin check. Which makes sense.
Attachment #8542842 - Attachment is obsolete: true
Attachment #8547027 - Flags: review?(sstamm)
Comment on attachment 8547027 [details] [diff] [review] Test case for comment #0 Review of attachment 8547027 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, Alex. Some minor comments... this isn't quite exactly testing the situation in comment 0. This is testing: [a [b c>c] [b c>a]] (a is triggering, is referrer) As I understand it, comment 0 is asking for: [a [a a>a] [b b>b]] (a is triggering, is referrer) ::: dom/base/test/test_bug1091883.html @@ +13,5 @@ > +<body> > +<p><a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1091883">Mozilla Bug 1091883</a></p> > + > +<iframe src="http://example.org/tests/dom/base/test/file_bug1091883_frame.html"></iframe> > +<iframe src="http://example.org/tests/dom/base/test/file_bug1091883_frame.html"></iframe> Nit: these iframes should have an id attribute so it's easy to pick one out individually. @@ +22,5 @@ > +// Navigate both subframes to the target. > +window.onload = function() { > + var target = "/tests/dom/base/test/file_bug1091883_target.html"; > + frames[0].frames[0].location = "http://mochi.test:8888" + target; > + frames[1].frames[0].location = "http://example.com" + target; You should make one of these two same-origin as the parent frame (example.org, not example.com). The thing we're checking is that this triggering principal (mochi.test:8888) is used when checking cross-origin and not the parent frame (which will always be cross-origin for these two targets since it is example.com). @@ +35,5 @@ > + "must be full referrer"); > + } else { > + is(event.data, "http://mochi.test:8888", "must be origin referrer"); > + } > + if (++numTests == 2) { Nit: >= 2 (just in case we over-count due to new prefetch implementation or something). Better yet, record the result for both tests individually and test them both before calling SimpleTest.finish() to make sure that we didn't just check the same one twice.
Attachment #8547027 - Flags: review?(sstamm) → review-
Attached patch Test cases (obsolete) (deleted) — — Splinter Review
Added tests for all 3^3=27 combinations of M [X [Y]] -> Z. M is fixed and (X, Y, Z) take on all possible values. Since it's M that navigates to Z, the referrer needs to be determined by whether the origins of M and Z match, irrespective of the origins of X and Y. One could conceivably mess this up in a number of different ways, so I just covered all possible cases. Addressed all other review comments as well.
Attachment #8551645 - Flags: review?(sstamm)
Attachment #8547027 - Attachment is obsolete: true
Comment on attachment 8551645 [details] [diff] [review] Test cases Review of attachment 8551645 [details] [diff] [review]: ----------------------------------------------------------------- Sorry this took so long. Yes, looks good. It's a big test, though, we should run it through try to make sure it will work on Android, B2G, etc.
Attachment #8551645 - Flags: review?(sstamm) → review+
Alex, since you did all the work here I am assigning this one to you :-)
Assignee: mozilla → averstak
Thanks, Sid. May I ask you to submit a try run? I don't have committer access.
Flags: needinfo?(sstamm)
Attached patch Test cases (deleted) — — Splinter Review
Trivial rebase, carrying over r+ from sstamm. Try run looks fine - no timeouts.
Attachment #8551645 - Attachment is obsolete: true
Attachment #8571082 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/ced7bcaa0e52
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
I'm not sure this test case does the right thing. > Concretely, say you have two pages: page A at http://a.foo.com and page B at > http://b.foo.com. Now if A triggers a location set on a subframe on B, say > to http://b.foo.com/subframe, the referrer will be http://a.foo.com. The > question is whether this is a cross-origin request or not. What is the > request origin in this situation per the spec? Assume you are on a page on http://mochi.test:8888. That page has an iframe to a.foo.com and b.foo.com. b.foo.com further has a grandchild frame http://b.foo.com/subframe. a.foo.com can navigate the grandchild b.foo.com frame if document.domain is set to foo.com. Once a.foo.com navigates the grandchild frame, we should check the referrer to see what it is (is it the origin only because this is considered a cross origin load or is it the full url). Looking at this test, it seems like it's using example.com and example.org, so we can't use the document.domain trick. But, example.com shouldn't be able to navigate an example.org iframe per https://bugzilla.mozilla.org/show_bug.cgi?id=408052, right? So how does this test work?
Sid, have you had a chance to take another look at this? need-info'ing you.
Flags: needinfo?(sstamm)
(In reply to Tanvi Vyas [:tanvi] from comment #17) > So how does this test work? Apparently, it doesn't. :-/ It tests a related but different case. The triggering principal is different from the loading principal in the checked-in test too; but this test doesn't cover what happens on assignment to document.domain. File a followup?
(In reply to Alex Verstak from comment #19) > (In reply to Tanvi Vyas [:tanvi] from comment #17) > > So how does this test work? > > Apparently, it doesn't. :-/ It tests a related but different case. The > triggering principal is different from the loading principal in the > checked-in test too; but this test doesn't cover what happens on assignment > to document.domain. > > File a followup? Since the example in comment 0 of this bug uses the same top level domain and describes one frame/document navigating a subframe of another frame/document, I'm inclined to reopen this bug and add another test instead of opening a new bug. I do wonder if BZ actually meant separate documents in separate windows/tabs or if separate frames on the same window are sufficient.
(In reply to Tanvi Vyas [:tanvi] from comment #20) > I do wonder if BZ actually meant separate documents in separate windows/tabs > or if separate frames on the same window are sufficient. There's no real argument against writing tests to cover more scenarios... as long as someone has time to do that, of course. :-)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee: averstak → nobody
Franziskus you wanna take on this bug?
Flags: needinfo?(franziskuskiefer)
Assignee: nobody → franziskuskiefer
Flags: needinfo?(franziskuskiefer)
(In reply to Tanvi Vyas [:tanvi] from comment #17) > Assume you are on a page on http://mochi.test:8888. That page has an iframe > to a.foo.com and b.foo.com. b.foo.com further has a grandchild frame > http://b.foo.com/subframe. a.foo.com can navigate the grandchild b.foo.com > frame if document.domain is set to foo.com. Once a.foo.com navigates the > grandchild frame, we should check the referrer to see what it is (is it the > origin only because this is considered a cross origin load or is it the full > url). After testing this it seems that setting document.domain (which is necessary in order to navigate the other iframe) messes with the same origin check. The current behaviour is that we get a full referrer even when setting a origin-when-cross-origin policy. I would expect to get origin, i.e. that this request is considered cross origin. Maybe bz can comment here what the expected behaviour should be.
Flags: needinfo?(bzbarsky)
What does the spec say? Which origins are supposed to be compared for determining the "cross-origin" part of origin-when-cross-origin? Also, I'm a bit surprised you needed to set document.domain to navigate the other iframe. Generally a page can navigate its own subframes and their subframes no matter what origins they all come from.
Flags: needinfo?(bzbarsky)
ok, so this actually seems correct per spec [1] as document.domain changes the effective origin such that the request is not cross-origin after setting document.domain to the same domain. We have to set document.domain because frame1 is navigating the subframe in frame2, not the page. Not setting document.domain throws NS_ERROR_DOM_PROP_ACCESS_DENIED. [1] https://html.spec.whatwg.org/multipage/browsers.html#origin
document.domain changes the effective script origin but does not change the origin. Is the determination of "cross-origin" involved here supposed to use the effective script origin or the origin? > We have to set document.domain because frame1 is navigating the subframe in frame2, not > the page. So what? > Not setting document.domain throws NS_ERROR_DOM_PROP_ACCESS_DENIED. How are you navigating, exactly? Setting window.location should work cross-origin. So should setting window.location.href and calling window.location.replace(). Getting your hands on subframes is totally doable cross-origin as well. So which exact operation is throwing NS_ERROR_DOM_PROP_ACCESS_DENIED for you?
Flags: needinfo?(franziskuskiefer)
To have something to discuss I made a test with the following setup: www.myzilla.cf contains two iframes test1.myzilla.cf/frame1 and test2.myzilla.cf/frame2, which contains another iframe test2.myzilla.cf/subFrame. The link clickme in frame1 sets parent.frames[1].frames[0].location = "http://test2.myzilla.cf/subFrame2.html". [1] has no document.domain set -> clickme link does not work (NS_ERROR_DOM_PROP_ACCESS_DENIED) [2] has document.doman set to myzilla.cf in frame1 and subFrame -> click me link works Chrome has the same behaviour btw. and has a more explanatory error message: "Unsafe JavaScript attempt to initiate navigation for frame with URL 'http://test2.myzilla.cf/subFrame2.html' from frame with URL 'http://test1.myzilla.cf/frame1WDD.html#'. The frame attempting navigation is neither same-origin with the target, nor is it the target's parent or opener." Regarding referrer: it is set to frame1 (this is in contrast to Chrome, which uses frame2 as referrer) [1] http://www.myzilla.cf/iframeTest.html [2] http://myzilla.cf/iframeTestWithDocumentDomain.html
Flags: needinfo?(franziskuskiefer)
Ah, I see. You're running into the check in nsDocShell::CheckLoadingPermissions, which happens after the location setter has actually been called. If your frame 1 were same-origin with frame 2 (but not with "subframe"), or frame 1 were same-origin with the main page (but not necessarily either frame 2 or subframe), then you could do that location assignment without doing any document.domain setting. It's worth creating a testcase like that which doesn't use document.domain and then see what the referrer ends up being in that situation.
Added two more test sites (without any change of document.domain): [1] frame 1 and frame 2 are same origin -> navigation works [2] frame 1 is same origin as main page -> navigation works [1] http://www.myzilla.cf/iframeTest2.html [2] http://www.myzilla.cf/iframeTest3.html
The referrer in those cases without setting document.domain is the correct url of frame 1
(In reply to Boris Zbarsky [:bz] from comment #26) > document.domain changes the effective script origin but does not change the > origin. Is the determination of "cross-origin" involved here supposed to > use the effective script origin or the origin? > I believe it is supposed to use the origin. So we are not honoring the referrer policy if the load is same effective script origin but not same origin. > Regarding referrer: it is set to frame1 (this is in contrast to Chrome, > which uses frame2 as referrer) > It is actually also set to frame1 in Chrome. The next question is what happens in Chrome with this example http://myzilla.cf/iframeTestWithDocumentDomain.html plus a referrer policy of origin-when-cross-origin.
> I believe it is supposed to use the origin. OK, then setting document.domain should have no effect per spec and if it does we have a bug.
(In reply to Tanvi Vyas [:tanvi] from comment #31) > (In reply to Boris Zbarsky [:bz] from comment #26) > > document.domain changes the effective script origin but does not change the > > origin. Is the determination of "cross-origin" involved here supposed to > > use the effective script origin or the origin? > > > I believe it is supposed to use the origin. So we are not honoring the > referrer policy if the load is same effective script origin but not same > origin. > > The next question is what happens in Chrome with this example > http://myzilla.cf/iframeTestWithDocumentDomain.html plus a referrer policy > of origin-when-cross-origin. Here is Franziskus' test case that sets origin-when-cross-origin http://myzilla.cf/iframeTestWithDocumentDomainRP.html : www.myzilla.cf contains two iframes test1.myzilla.cf/frame1 and test2.myzilla.cf/frame2, which contains another iframe test2.myzilla.cf/subFrame. The link clickme in frame1 sets parent.frames[1].frames[0].location = "http://test2.myzilla.cf/subFrame2.html". document.domain is set on frame1 and the subframe. The page has the meta referrer set to origin-when-cross-origin. In Firefox and Chrome, we also get the full referrer when we click "click me" and navigate the subframe of iframe2. Mike, is this expected behavior? Should the referrer policy spec consider same effective script origin in addition to same origin? https://html.spec.whatwg.org/multipage/browsers.html#origin https://w3c.github.io/webappsec/specs/referrer-policy/#same_origin-request
Flags: needinfo?(mkwst)
I don't think we should consider the effective script origin in any new places. It's a legacy artifact for document.domain. There might be a problem with the specifications though. In particular, per https://fetch.spec.whatwg.org/ requests only have on associated principal/global (called client) whereas we seem to have two (triggering / loading).
Assignee: franziskuskiefer → nobody
Priority: -- → P5
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: