Closed Bug 390168 Opened 14 years ago Closed 14 years ago

[FIX][Trunk] Secure (encrypted, https) sites loading as being partially encrypted. Broken lock is present as is the white address bar

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9alpha8

People

(Reporter: stevee, Assigned: bzbarsky)

References

()

Details

(Keywords: regression)

Attachments

(3 files)

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a7pre) Gecko/2007072905 Minefield/3.0a7pre ID:2007072905

1. New profile, start firefox
2. Go to https://gmail.google.com - Note location bar is yellow (secure)
3. "You have requested an encrypted page. <etc>" dialog appears, click OK
4. Enter username and password
5. Click "Sign In" (Doesn't seem to matter if you tick 'Remember me on this computer', but leave this unticked anyway.)
6. "Do you want minefield to remember this password?" dialog. Choose "Not Now" (Doesn't seem to matter what option you choose here anyway)
7. Gmail page loads. Location bar is yellow (secure)
8. In the location bar, left click the rss feed icon.
9. "You are about to leave an encrypted page. <etc>" dialog. Click OK.
10. The rss subscription page should be present. Note the location bar is white (not secure).
11. Click the back button.
12. "You have requested an encrypted page that contains some unencrypted information. <etc>". Click OK.
13. Gmail page loads. It should now be a half-and-half page. Note the location bar is not yellow, and the icon of the lock with the red slash through it is visible.

Works (gmail page is marked secure after going to rss feed subscription page, then back):
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a5pre) Gecko/2007050813 Minefield/3.0a5pre

Broken (gmail page is marked half-and-half after going to rss feed subscription page, then back):
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a5pre) Gecko/2007050915 Minefield/3.0a5pre

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-05-08+12&maxdate=2007-05-09+16&cvsroot=%2Fcvsroot

Bug 383369 covers a manifestation of this bug on the branch, but these reproduction steps don't work on branch, just on trunk.
Flags: blocking-firefox3?
Does this only happen when feeds are involved?

Related to bug 337897?
Hmm. I've only been able to manage to reliably reproduce the problem by invoking the rss subscription page and then hitting back

But at the same time i've also had secure sites loose their yellow location bar background without any rss feed usage. (eg: gmail, bugzilla). Unfortunately i've not worked out any steps to reproduce the problem without using rss feed subscription. (If any steps do exist, they may also work on branch, re: bug 383369)
Flags: blocking-firefox3? → blocking-firefox3+
Is there some evidence for bug  255820 being the problem?  Or is it just a matter of "most likely thing in the range" (with which I agree, fwiw)?

I know with gmail it's a long shot, but is there a simplified (even slightly) testcase around?

Maybe we stopped doing a charset reload in this case and hit a bug in PSM with wyciwyg or something?

Oh, and I'm sure this is in the wrong product (and hence has the wrong blocking flags).  This is so _not_ a Firefox UI bug.  It's either a bug in Gecko or a bug in PSM.
So as a simple test:

1)  Load this page.
2)  Type "javascript:document.write('aaa'); document.close()" in the URL bar and
    hit enter.
3)  Load any other page.
4)  Hit the back button

loses the lock icon.  Is gmail hitting something like that?
OK.  See comment 4.  Is gmail using document.write (they have to be to be affected by that checkin) and did we stop doing charset reloads on it?

Put another way, did the scenario in comment 4 also regress then, or was it broken all along?
(In reply to comment #4)
> So as a simple test:
> 
> 1)  Load this page.
> 2)  Type "javascript:document.write('aaa'); document.close()" in the URL bar
>     and hit enter.
> 3)  Load any other page.
> 4)  Hit the back button
> 
> loses the lock icon.  Is gmail hitting something like that?

I'm not sure. I get the same behaviour in Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a5pre) Gecko/2007050813 Minefield/3.0a5pre and in Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a5pre) Gecko/2007050915 Minefield/3.0a5pre. (The same behaviour being that in step (2) the secure site icon is present and the location bar yellow, and in step (4) the secure site icon has completely disappeared and the location bar is white. So, no change between the regression range in comment 0, and also no half-and-half security icon.)
(In reply to comment #4)
> So as a simple test:
> 
> 1)  Load this page.
> 2)  Type "javascript:document.write('aaa'); document.close()" in the URL bar
> and
>     hit enter.
> 3)  Load any other page.
> 4)  Hit the back button
> 
> loses the lock icon.  Is gmail hitting something like that?
> 
in Gmail
open Gmail
open any page from bookmark (even works with https://mail.google.com/mail/) in the same tab.
hit the back button.
Peter, your comment doesn't answer my question.

Steve, gmail's behavior is consistent with having something like my testcase in an iframe, where the main page is secure, but the iframe now isn't due to a bug in PSM's handling of wyciwyg URIs.  And the behavior could have changed based on whether we do a charset reload or not, basically.

In any case, I will wager that fixing the test in comment 4 will fix gmail too.
Attached image screenshot
from Page Info -> Security before(secure) and after
Blocks: 358438
Another question.  Does disabling bfcache affect behavior either on gmail (doubtful) or on my test?
browser.sessionhistory.max_total_viewers = 0 (bfcache off)

fixes the broken lock/white locationbar in Gmail

doesn't fix your testcase
Really?  Well that's just _interesting_.  ;)  Sounds like something similar to bug 358438, possibly...

I suppose I could try debugging to figure out exactly why my changes changed behavior here, but given the general breakage bfcache induces on the lock icon I'd rather we fixed that and then checked what this case looks like.
For me, and the repo steps and builds in comment 0, setting browser.sessionhistory.max_total_viewers = 0 makes no different; the 2007050813 build still shows the full security lock when logging into gmail, clicking the rss feed, and clicking back.. and the 2007050915 build still shows the half-and-half security icon when logging into gmail, clicking the rss feed, and clicking back.
I tweaked the tracing for nsSecureBrowserUI a bit, in order to find out which request triggers the switch to insecure.

We get a request for wyciwyg://1/https://mail.google.com/mail/?view=page&name=js&ver=1t6ksswaq6lz2

PSM can QI this to nsIHttpChannel, so it concludes: do not ignore

When PSM tries to obtain the transport-security-info, it fails, and concludes it's an insecure request.
After bug 255820 it writes 3 prefs to prefs.js: 
user_pref("security.warn_entering_secure", false);
user_pref("security.warn_leaving_secure", false);
user_pref("security.warn_viewing_mixed", false);
It didn't do that before.
Er...  How does that happen, exactly?  The channel for that URI should be an nsWyciwygChannel, which does not QI to nsIHttpChannel.

What's the channel concrete class there?  What's the callstack?  Which of the channel URIs is the wyciwyg: URI?
(In reply to comment #17)
> Er...  How does that happen, exactly?  The channel for that URI should be an
> nsWyciwygChannel, which does not QI to nsIHttpChannel.

Yes yes, you're right, sorry. It's a nsWyciwygChannel :-)

OK.  And PSM doesn't ignore those?

I guess those are supposed to provide a securityInfo, right?
The other portions of my comment 15 are correct.

Yes, the channel is indeed a nsWyciwygChannel. PSM wants to obtain an nsITransportSecurityInfo, but fails => insecure
For what it's worth, my firefox build crashe with a completely corrupted stack after step 8 from comment 0.
Kai, if you can actually debug this... can you try breakpointing SetSecurityInfo, GetSecurityInfo, and ReadFromCache() in nsWyciwygChannel and see what's going on?
(In reply to comment #16)
Sorry for the stupid comment. I'm not quite fresh :/.
(In reply to comment #4)
> So as a simple test:
> 
> 1)  Load this page.
> 2)  Type "javascript:document.write('aaa'); document.close()" in the URL bar
> and
>     hit enter.
> 3)  Load any other page.
> 4)  Hit the back button
> 
> loses the lock icon.  Is gmail hitting something like that?


Boris, if I understand correctly you're saying:

1) load https://bugzilla.mozilla.org/show_bug.cgi?id=390168
2) load "javascript:document.write('aaa'); document.close()"
3) load mozilla.org
4) back button

When I do the above, hitting back button "only once", I confirm, I do not get the lock icon. I do not a yellow bar.

But the displayed content I get is "aaa". It seems to behave as expected?

When I hit the back button a second time, I arrive back at the bugzilla page, with a good lock and a yellow url bar.

Do you get something different?

Where is the bug in your scenario?
Let me say I'm using a trunk build from last week, let me update, in case this is related to very recent changes.
> But the displayed content I get is "aaa". It seems to behave as expected?

No.  Things should look identical after step 2 and step 4.  They do not.  After step 2, I see a lock and yellow bar, and the document.written content (secure, since written from a secure page).  After step 4, I see no lock and no yellow url bar.  The content in the page is the same, of course.
So I tried to debug this, but I think I'm not the right person. Someone with understanding of bfcache should debug it. Or, anyone could debug this, because it's easily reproducible.

So here is my experience and setup (on linux), using the latest trunk sources from today:

- startup with gdb
- allow the browser to load the start page, 
  so that all debugging symbols are loaded
- break into debugger
- set multiple breakpoints, I used:

break 'nsWyciwygChannel::GetSecurityInfo'
break 'nsWyciwygChannel::WriteToCacheEntry'
break 'nsWyciwygChannel::SetSecurityInfo'
break 'nsWyciwygChannel::ReadFromCache'
break 'nsWyciwygChannel::nsWyciwygChannel'
(all)

- continue


And my test case was:
- load start page (http)
- log in to gmail using https
- go to http page
- use back


I tried the above in multiple separate sessions.

I tried having set browser.sessionhistory.max_total_viewers to -1 (default, bfcache enabled, as you say)
and I tried having it set to 0 (bfcached disabled, as you say)


I get a very surprising behaviour.

In some sessions the breakpoints are hit. I saw this with bfcache enabled or disabled!

In some sessions none of the breakpoints were hit. Again, I saw this with bfcache enabled or disabled!

Are you really sure this is controlled by the above config flag? (I restarted after having set the config, before trying the testcase)

Or does my debugger suck?


Whenever I hit the WriteToCacheEntry, I got a good mSecurityInfo.

Whenever I hit the ReadFromCache, I got a NULL for mSecurityInfo.
Ok.  With my testcase, I can reproduce this, and this is in fact the fault of bug 255820.  Kai, thank you for digging into this!  It was incredibly helpful in getting me started in the right direction.

Patch coming up.
Attached patch Proposed fixSplinter Review
So we were opening the cache entry before we had our "don't cache on disk" channel flags set... and disk cache can't store securityInfo, apparently.

I could reorder the code that sets the charset with the code that sets the flags, but this seems more robust.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #274723 - Flags: superreview?(jst)
Attachment #274723 - Flags: review?(jst)
I'd like to apologize to everyone for not digging into this earlier, by the way.  Just pretty swamped right now. :(
Component: Security → DOM
Flags: blocking-firefox3+
OS: Windows 2000 → All
Product: Firefox → Core
Hardware: PC → All
Summary: [Trunk] Secure (encrypted, https) sites loading as being partially encrypted. Broken lock is present as is the white address bar → [FIX][Trunk] Secure (encrypted, https) sites loading as being partially encrypted. Broken lock is present as is the white address bar
Target Milestone: --- → mozilla1.9 M8
Flags: blocking1.9+
(In reply to comment #29)
> disk cache can't store securityInfo, apparently.

Does it mean that browser.cache.disk_cache_ssl=true case will not be fixed by this patch and, for that matter, will never be fixed?

No big deal I suppose.
If I read the code right, browser.cache.disk_cache_ssl=true would make this problem appear for all ssl pages. Fixing it would require serializing the security info to disk, and all the cache has right now is an opaque pointer..
Attachment #274723 - Flags: superreview?(jst)
Attachment #274723 - Flags: superreview+
Attachment #274723 - Flags: review?(jst)
Attachment #274723 - Flags: review+
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
Flags: in-litmus?
Attached patch Followup fixSplinter Review
That patch broke the test for bug 255820.  What happens there is that the <meta> tag makes us call SetCharsetAndSource _after_ we've started writing to cache.  This patch makes us handle that.

I've already checked this in to fix the orange....  This applies on top of the previous patch.
Attachment #278156 - Flags: superreview?(jst)
Attachment #278156 - Flags: review?(jst)
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007082500 Minefield/3.0a8pre ID:2007082500

VERIFIED
Attachment #278156 - Flags: superreview?(jst)
Attachment #278156 - Flags: superreview+
Attachment #278156 - Flags: review?(jst)
Attachment #278156 - Flags: review+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.