Closed Bug 664721 Opened 13 years ago Closed 13 years ago

1st party iframe request treated as 3rd party when initiated by and replacing a 3rd party origin

Categories

(Core :: Networking: Cookies, defect, P2)

x86
Windows XP
defect

Tracking

()

RESOLVED FIXED
mozilla8
Tracking Status
firefox5 --- affected
firefox6 --- affected
firefox7 --- fixed

People

(Reporter: al_9x, Assigned: bzbarsky)

References

Details

(Keywords: regression, Whiteboard: [qa?])

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Build Identifier: 

https://groups.google.com/group/mozilla.dev.apps.firefox/browse_thread/thread/7e2323e6d094cbac

I've reproduced this issue with a simple synthetic test, but I'll describe
in terms of the real site where I encountered it.

Verizon wireless (VZW) bill payment site has an iframe which loads
verifiedbyvisa.com.  The visa page onload, replace navigates the iframe (as opposed to a sub request) back to VZW.  With 3rd party cookies disabled, this
VISA->VZW replacement request sends VZW cookies in Fx 3.6.17, but not in
4.0.1 where the transaction fails.

Why this change?  I think Fx3.6 behavior is correct.  For a request to be
considered 3rd party, it has to be a subrequest of a 1st party navigation,
but here VZW replaces VISA it's not a sub-request to the VISA request.  If
it's to be considered a sub-request at all, it would be of the parent page,
which is VZW, so it's a 1st party sub-request.


Reproducible: Always
A pointer to the synthetic test (or a regression range, or ideally both) would be much appreciated... As things stand, it's hard to tell what's actually going on here.
(In reply to comment #1)
> A pointer to the synthetic test (or a regression range, or ideally both)
> would be much appreciated... As things stand, it's hard to tell what's
> actually going on here.

A synthetic test is very simple:

<html>
<head>
<script>
document.cookie = 'a=b';
</script>
</head>
<body>
<iframe src="http://different-domain/iframe_3rd.html"></iframe>
</body>
</html>

iframe_3rd.html - onload navigates to iframe_1st.(asp(x)|php) on the original domain
iframe_1st - dumps out cookies via the server script
_____________________________________________________________

Last good nightly: 2010-10-19 First bad nightly: 2010-10-20
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c9df0c5cbf8c&tochange=7aa9763e9d41
_____________________________________________________________

I have a problem with mozregression on my machine.  The Firefox process that it starts does not open the main window, just sits there:
bash.exe -> bash.exe -> mozregression.exe -> python.exe -> firefox.exe
If I close the terminal, all the processes except for Firefox terminate and then the Firefox window opens.  Any idea how to troubleshoot/resolve this?
The reason is probably that the third party detection code changed. Instead of comparing the request URL with the toplevel URL, we now compare it with each URL up the docshell tree. Since iframe_3rd is third-party, so is the request. We did this because that's the behavior we wanted for IndexedDB, and I couldn't think of a reason to not do it for cookies.

Is iframe_3rd actually replaced by a load on the original domain? Or does it simply perform a request to it? If the former, this might be an implementation bug. I'm not sure.
(In reply to comment #3)
> The reason is probably that the third party detection code changed. Instead
> of comparing the request URL with the toplevel URL, we now compare it with
> each URL up the docshell tree. Since iframe_3rd is third-party, so is the
> request. We did this because that's the behavior we wanted for IndexedDB,
> and I couldn't think of a reason to not do it for cookies.
> 
> Is iframe_3rd actually replaced by a load on the original domain? Or does it
> simply perform a request to it? If the former, this might be an
> implementation bug. I'm not sure.

it's the former, a replacement, iframe_3rd is not a parent of iframe_1st

iframe_3rd:

<body onload="location = 'http://original_domain/iframe_1st.asp'">
</body>
____________________________________

I don't see how this can be considered a 3rd party request.
Is that supposed to change the document URI? bz?
(Specifically, the URI of the script object principal of the window: http://mxr.mozilla.org/mozilla-central/source/content/base/src/ThirdPartyUtil.cpp#123)
That assignment should navigate that iframe to http://original_domain/iframe_1st.asp.  So yes, it would change the URI of the script object principal of the window ... after we get a response from the server.
Please NEW this.  Any updates?
Status: UNCONFIRMED → NEW
Ever confirmed: true
So what happened with this investigation?
Boris and Dan, I gave you what you asked for, a synthetic test and a regression range, two weeks ago.  What is preventing you from assessing this problem?  Did you find the cause?  Was the change deliberate or accidental? Are you going to fix it?
The main thing preventing me is that this is not my area of expertise and I'm swamped with stuff I'm supposed to work on.

As for Dan, he's doing cookie stuff part-time at best at the moment.

dwitte, would it make sense to not compare document requests with the URI of their own docshell?
Dan, how does this look?  How can I write a test for this?  I can't make sense of the tests from bug 595305...
Attachment #543046 - Flags: review?(dwitte)
I pushed that change to try: http://tbpl.mozilla.org/?tree=Try&rev=f38f9e0b1675

al_9x, there should be a Windows build with that change that you can test to make sure it fixes your situation sometime tomorrow.  I'll comment here with the URL to it once it exists.
Assignee: nobody → bzbarsky
Whiteboard: [need review]
Priority: -- → P2
(In reply to comment #14)
> al_9x, there's a test build at
> https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bzbarsky@mozilla.
> com-f38f9e0b1675/try-win32/ if you want to try it out.

verified on the test and VZW
Summary: 1st party request treated as 3rd party → 1st party iframe request treated as 3rd party when initiated by and replacing a 3rd party origin
Great.  thank you for checking that!
It seems dwitte is not available to review this, can anyone else do it?
dwitte told me he plans to review this.  No one else is really qualified to do it, no.
Comment on attachment 543046 [details] [diff] [review]
Treat a document load in a subframe as first-party if it's first-party with all _ancestors_ of the subframe, no matter what's loaded in the subframe itself.

Looks good. r=dwitte!
Attachment #543046 - Flags: review?(dwitte) → review+
Whiteboard: [need review] → [need landing]
Pushed http://hg.mozilla.org/integration/mozilla-inbound/rev/cedefaaaaceb

al_9x, if nothing goes wrong this should ship around November 8 or so.
Whiteboard: [need landing]
Target Milestone: --- → mozilla8
Flags: in-testsuite?
(In reply to comment #20)
> Pushed http://hg.mozilla.org/integration/mozilla-inbound/rev/cedefaaaaceb
> 
> al_9x, if nothing goes wrong this should ship around November 8 or so.

Would that be 8.0? I am curious, why not 7.0?
That would be 8.0, yes.  7.0 is in stabilization, and I'm not convinced that this bug is serious enough to land on aurora.

That said, the fix is pretty simple and I think safe.  If you want to make the argument that this should land on aurora, please nominate the patch for aurora approval with a comment explaining why it's worth landing there.
(In reply to comment #22)
> If you want to make
> the argument that this should land on aurora, please nominate the patch for
> aurora approval

How is that done?

> with a comment explaining why it's worth landing there.

I would say that since this is a regression introduced in 4.0, it should be fixed as soon as possible.  If it never worked right, then I guess the normal flow would be ok.

With rapid releases, fixing regressions is going to be even more important, because sticking with an older working version will no longer be an option.
Comment on attachment 543046 [details] [diff] [review]
Treat a document load in a subframe as first-party if it's first-party with all _ancestors_ of the subframe, no matter what's loaded in the subframe itself.

> How is that done?

Load https://bugzilla.mozilla.org/attachment.cgi?id=543046&action=edit then scroll down to "flags" and change the "approval‑mozilla‑aurora" flag to the value "?".

That said, you may not have the bits to change those fields.  Can you actually edit them?

That said, nominating per comment 23.  As I said, this is a simple fix, and this fixes at least one known site that is unusable due to this problem if third party cookies are disallowed.
Attachment #543046 - Flags: approval-mozilla-aurora?
(In reply to comment #24)
> 
> Load https://bugzilla.mozilla.org/attachment.cgi?id=543046&action=edit then
> scroll down to "flags" and change the "approval‑mozilla‑aurora" flag to the
> value "?".
> 
> That said, you may not have the bits to change those fields.  Can you
> actually edit them?
> 

No.
Keywords: regression
Attachment #543046 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
http://hg.mozilla.org/mozilla-central/rev/cedefaaaaceb
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
I just stumbled across this bug. While I haven't checked the solution yet, this seems to fix a very widely reported problem with 'Verified by Visa' and 'Mastercard Securecode' payment authorization not working in Firefox 4+.

I'm not sure how widespread these payments schemes are in the States, but over here in UK a most online retailers use them as a mandatory part of the payment process. The majority of these websites DO NOT work in Firefox due to this bug, affecting potentially thousands of users.

Here's a quick list of bugs which appear to be duplicates of this one:

Bug #655386
Bug #598559
Bug #672469

As well as various reports in forums, such as:

http://support.mozilla.com/en-US/questions/805248
http://support.mozilla.com/en-US/questions/810828
One of the comments at http://forums.theregister.co.uk/forum/1/2011/02/21/firefox_4_beta_12_delay/

I would therefore strongly suggest that this fix be considered for inclusion in Firefox asap.
tvk, this bug is only relevant when third-party cookies are disallowed.  The default setting is to allow them.  So this bug only affects situations when a user has changed cookie preferences from the defaults.


Note that some sites actually depend on third-party cookies being set in situations that really are third-party to work.  This fix won't help those.  That's why the default setting is to accept third-party cookies.  Let's follow up with your specific issue in bug 672469, because it may be quite different from both this bug and the other two bugs you cite.
Blocks: 655386
Mozilla/5.0 (X11; Linux x86_64; rv:7.0) Gecko/20100101 Firefox/7.0

Could anyone provide a clear set of STRs in order to get this issue verified in QA?
See comment 2?
(In reply to al_9x from comment #2)
> 
> I have a problem with mozregression on my machine.  The Firefox process that
> it starts does not open the main window, just sits there:
> bash.exe -> bash.exe -> mozregression.exe -> python.exe -> firefox.exe
> If I close the terminal, all the processes except for Firefox terminate and
> then the Firefox window opens.  Any idea how to troubleshoot/resolve this?

Though it's off topic, since I posted this problem, I might as well post the solution:

The cause is environment variable CYGWIN=tty

"CYGWIN=ntsec tty" is recommended by the following openssh on windows tutorial:
http://pigtail.net/LRP/printsrv/cygwin-sshd.html

for the following reason:

"tty is an environment variable used by cygwin to make it work properly with editors such as pico and nano
Without it (the default case), you won't be able to insert characters."
http://pigtail.net/LRP/printsrv/tty.html

Here's what happens to mozregression when "CYGWIN=tty" is set:

1. there are no progress messages in the shell window:

Downloading nightly from <date>

Starting nightly

Was this nightly good, bad, or broken?

2. the firefox.exe process starts but its window doesn't open

Without "CYGWIN=tty", it works.
Blocks: 672469
qa?: is QA fix verification needed on this bug? if so, is comment 2 the test to verify?
Whiteboard: [qa?]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: