Last Comment Bug 664721 - 1st party iframe request treated as 3rd party when initiated by and replacing a 3rd party origin
: 1st party iframe request treated as 3rd party when initiated by and replacing...
Status: RESOLVED FIXED
[qa?]
: regression
Product: Core
Classification: Components
Component: Networking: Cookies (show other bugs)
: unspecified
: x86 Windows XP
: P2 normal (vote)
: mozilla8
Assigned To: Boris Zbarsky [:bz] (Out June 25-July 6)
:
Mentors:
: 672469 (view as bug list)
Depends on:
Blocks: 655386 672469
  Show dependency treegraph
 
Reported: 2011-06-16 07:58 PDT by al_9x
Modified: 2011-09-22 15:18 PDT (History)
8 users (show)
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
affected
fixed


Attachments
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. (2.75 KB, patch)
2011-06-29 20:40 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
dwitte: review+
asa: approval‑mozilla‑aurora+
Details | Diff | Review

Description al_9x 2011-06-16 07:58:53 PDT
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
Comment 1 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-06-16 08:44:53 PDT
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.
Comment 2 al_9x 2011-06-16 10:16:25 PDT
(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?
Comment 3 dwitte@gmail.com 2011-06-16 11:04:44 PDT
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.
Comment 4 al_9x 2011-06-16 11:27:07 PDT
(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.
Comment 5 dwitte@gmail.com 2011-06-16 11:33:44 PDT
Is that supposed to change the document URI? bz?
Comment 6 dwitte@gmail.com 2011-06-16 11:35:30 PDT
(Specifically, the URI of the script object principal of the window: http://mxr.mozilla.org/mozilla-central/source/content/base/src/ThirdPartyUtil.cpp#123)
Comment 7 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-06-16 11:52:29 PDT
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.
Comment 8 al_9x 2011-06-17 05:59:13 PDT
Please NEW this.  Any updates?
Comment 9 al_9x 2011-06-23 17:28:39 PDT
So what happened with this investigation?
Comment 10 al_9x 2011-06-29 16:41:26 PDT
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?
Comment 11 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-06-29 19:14:22 PDT
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?
Comment 12 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-06-29 20:40:34 PDT
Created 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.

Dan, how does this look?  How can I write a test for this?  I can't make sense of the tests from bug 595305...
Comment 13 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-06-29 20:43:22 PDT
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.
Comment 14 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-06-30 08:36:06 PDT
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.
Comment 15 al_9x 2011-06-30 09:10:51 PDT
(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
Comment 16 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-06-30 10:30:53 PDT
Great.  thank you for checking that!
Comment 17 al_9x 2011-07-12 17:52:23 PDT
It seems dwitte is not available to review this, can anyone else do it?
Comment 18 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-07-12 19:06:18 PDT
dwitte told me he plans to review this.  No one else is really qualified to do it, no.
Comment 19 dwitte@gmail.com 2011-07-18 21:38:54 PDT
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!
Comment 20 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-07-19 08:30:32 PDT
Pushed http://hg.mozilla.org/integration/mozilla-inbound/rev/cedefaaaaceb

al_9x, if nothing goes wrong this should ship around November 8 or so.
Comment 21 al_9x 2011-07-19 10:36:48 PDT
(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?
Comment 22 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-07-19 10:52:26 PDT
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.
Comment 23 al_9x 2011-07-19 11:41:27 PDT
(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 24 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-07-19 11:51:59 PDT
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.
Comment 25 al_9x 2011-07-19 12:23:15 PDT
(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.
Comment 26 Marco Bonardo [::mak] 2011-07-20 06:49:26 PDT
http://hg.mozilla.org/mozilla-central/rev/cedefaaaaceb
Comment 27 tvk 2011-07-21 07:13:51 PDT
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.
Comment 28 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-07-21 07:40:33 PDT
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.
Comment 29 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-07-22 08:40:28 PDT
http://hg.mozilla.org/releases/mozilla-aurora/rev/06fee5653ceb
Comment 30 Virgil Dicu [:virgil] [QA] 2011-08-19 08:34:07 PDT
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?
Comment 31 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-08-19 09:52:06 PDT
See comment 2?
Comment 32 al_9x 2011-08-19 15:42:01 PDT
(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.
Comment 33 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-08-23 13:58:25 PDT
*** Bug 672469 has been marked as a duplicate of this bug. ***
Comment 34 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-22 15:18:04 PDT
qa?: is QA fix verification needed on this bug? if so, is comment 2 the test to verify?

Note You need to log in before you can comment on or make changes to this bug.