Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1a2pre) Gecko/20080816032044 Minefield/3.1a2pre ID:20080816032044 When you visit a website which has an EV cert and you restart Firefox with session restore enabled the EV button will only be shown split a second. It gets reverted back to the gray button. This issue is only present on Windows. I'm not able to reproduce it on OS X. Steps to reproduce: 1. Start Firefox and enable Session Restore 2. Open https://www.paypal.com/ => Green EV button is shown 3. Restart Firefox and wait until the above site is restored => Gray button 4. Reload the page => Green EV button is visible again During step 3 you can see that the EV button is shown split a second and is immediately replaced by the gray button. Clicking on the gray button tells that no identity information is available. That's not true because after the reload you can see the identity information. Sometimes this also happens when opening such an URL the first time. I'll try to find the regression range for that issue.

This doesn't happen on 1.9.0. Regression window is from 20080713 - today. Will have a closer look now.

It fails between the following builds: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.2pre) Gecko/2008081601 GranParadiso/3.0.2pre ID:2008081601 Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1a2pre) Gecko/20080816032044 Minefield/3.1a2pre ID:20080816032044 Checkins: http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1218848940&maxdate=1218873059 I'm a bit confused. There is only one check-in (bug 423176) in this time frame which doesn't really seem to be raised this regression. CC'ing Marco who fixed bug 423176.

Sorry, I have to query hg instead of using Bonsai.

Same with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a2pre) Gecko/20080817025217 Minefield/3.1a2pre ID:20080817025217

hg checkins in this timeframe: http://hg.mozilla.org/mozilla-central/index.cgi/pushloghtml?startdate=2008-08-16+00%3A00%3A00&enddate=2008-08-16+05%3A00%3A00 The only bug which handles some security and image code is bug 135007. So cc'ing Kai and Honza.

confirming bug. For me, it's not necessary to use session restore. I start the browser, manually go to https://www.paypal.com, and see the reported problem. On reload, I get EV.

I think it's indeed very likely that this is caused by bug 135007. When the EV disappears, note that the status bar icon reports "broken lock icon" aka "mixed content".

I remember I noticed this problem (with broken icon) during fixing of that bug. However it seemed to me not happen from one repository update and I tough it were fixed then. Kai, I am free to take this bug, but not sooner then on Wednesday next week.

Thanks Kai. You are correct => Updating the summary and adding bug 135007 to blocker list.

This seems to be related to URIs like resource://gre/res/arrow.gif When I ignore state change events for those URIs, this bug is fixed. But we must solve two details: - right know I don't know of an efficient way to test whether a channel is a resource:// URI. The channel neither gives me an URI nor an OriginalURI. In my testing I use nsXPIDLCString reqname; aRequest->GetName(reqname); StringBeginsWith(reqname, NS_LITERAL_CSTRING("resource://")); - But that seems too expensive to me. - answer the question: is it always correct to ignore resource:// URIs for the security state of the document? In other words: Are such URIs always strictly local application data?

updating summary, because this is not limited to EV, it's for all https (where loading of the page triggers access to a resource:// image)

After talking with Kai on IRC we should make the summary a bit clearer.

again, not limited to EV

I agree. Unless it's some different issue, I see the mixed content indications problem displaying this very bug.

should we back it out? or check in the inefficient workaround?

Boris, Christian, can we assume that resource:// are always local trusted content, and thus can be ignored for tracking the secure state of a web page? I've been unable so far to find an efficient way to query a nsIRequest/nsIChannel to "resource://". See also comment 10 (no URI, no originalURI, can not use schemesis).

Any nsIChannel will always have a URI and an originalURI, unless its implementation is _very_ busted. If it's that busted, please treat it as untrusted no matter what. Imagelib nsIRequests are not nsIChannels, but they're imgIRequests, and provide the URI from that interface. resource: is basically equivalent to file: as far as the security manager is concerned. Once you have the nsIURI, you can test its chain for the URI_IS_LOCAL_FILE protocol bit. That will also handle jar:resource: for you, etc. Does that help?

Boris, that really helped very much! I'm using the following snippet to make a decision about resource:// and it works great to fix this bug. // ignore resource:// image URIs nsCOMPtr<nsIURI> uri; imgRequest->GetURI(getter_AddRefs(uri)); if (uri) { PRBool res; if (NS_SUCCEEDED(uri->SchemeIs("resource", &res)) && res) { isSubDocumentRelevant = PR_FALSE; } } This snippet patch overlaps / depends on the one in bug 135007. I used the ->schemeis approach, because that seemed easier. Are you ok with this patch? It might be less efficient than your proposal to test for the URI_IS_LOCAL_FILE bit. When you said "chain", I suppose I would have to use NS_URIChainHasFlags. That uses do_GetService each time, so I would have to cache the I/O service in each nsSecureBrowserUI object. If you think that would be better, I can adjust it.

I think caching an IOService in nsSecureBrowserUI and using the flag is the way to go. It'll be a little slower than the SchemeIs check, but a lot more correct and extensible.

Boris, either I'm doing something wrong, or your proposal does not work. The following snippet never gives me a "has flag" result. imgRequest->GetURI(getter_AddRefs(uri)); if (uri) { if (!mIOService) { mIOService = nsGetServiceByContractID(NS_IOSERVICE_CONTRACTID); } if (mIOService) { PRBool hasFlag; nsresult rv = mIOService->URIChainHasFlags(uri, nsIProtocolHandler::URI_IS_LOCAL_FILE, &hasFlag); if (NS_SUCCEEDED(rv) && hasFlag) { isSubDocumentRelevant = PR_FALSE; } } As the proposal from comment 18 works, should we do that?

You probably want to check for UI_RESOURCE too, not just LOCAL_FILE, no? That will cover chrome: and resource:.... (I thought resource: was LOCAL_FILE already, but clearly I was wrong; there are some XXX comments about this.)

We already have code that filters out nsIFileChannel and declares those as not relevant for sub content. So I guess it's sufficient to test for UI_RESOURCE, only, in order to fix this bug. With that change my testing shows it's fixed. This (unreal) patch contains some overlapping code, from bug 135007. The real merged patch is available over there, attachment 335640 [details] [diff] [review].

Basically the new portions in the attached patch are equivalent to comment 20, LOCAL_FILE => UI_RESOURCE, plus the code to cache the IOService object.

Comment on attachment 335641 [details] [diff] [review] Patch v1 >+++ b/security/manager/boot/src/nsSecureBrowserUIImpl.cpp >+ nsCOMPtr<nsINetUtil> aIOService; Name this ioService? >+ aIOService = nsGetServiceByContractID(NS_IOSERVICE_CONTRACTID); Is there a reason not to use do_GetService here? >+ // ignore resource:// image URIs Why just image URIs? Why not everything that's resource:// ?

(In reply to comment #24) > >+ nsCOMPtr<nsINetUtil> aIOService; > > Name this ioService? renamed... > >+ aIOService = nsGetServiceByContractID(NS_IOSERVICE_CONTRACTID); > > Is there a reason not to use do_GetService here? I had copied code from net-util. Changed to your preference... > >+ // ignore resource:// image URIs > > Why just image URIs? Why not everything that's resource:// ? I'm ok with that change from the functional point of view, but what about performance? Will we see a performance regression when we start calling ioService->URIChainHasFlags on every OnStateChange notification?

Updated patch to include Boris proposed changes. Again, contains some overlapping code, from bug 135007. The real merged patch is available over there, attachment 335692 [details] [diff] [review]

> but what about performance? That's a good question... I suspect that if we could replace the entire QI chain with a single URIChainHasFlags call that would be a performance win or at least not a loss. Otherwise, hard to say. But really, this should be a pretty fast call, so I doubt it would be noticeable.

In that last patch, why are we checking for an image request only when !httpRequest?

And seriously, I think that the code as it works right now (with interface-checking) on the channels is seriously broken: it will miss some types of insecure content, especially where extensions are involved. Please get a followup bug filed to use self-reported protocol flags or something instead of this QI approach?

(In reply to comment #29) > And seriously, I think that the code as it works right now (with > interface-checking) on the channels is seriously broken: it will miss some > types of insecure content, especially where extensions are involved. Please > get a followup bug filed to use self-reported protocol flags or something > instead of this QI approach? The current implementation is a hack. Rather than trying to put one band aid on top of the other, we must finally get bug 62178 done. I don't want to invent a new solution to detect the type of load that "already happened" when we really want to switch to an implementation that has the ability to actually block loading of insecure content.

OK. I can live with that. What about comment 28?

(In reply to comment #28) > In that last patch, why are we checking for an image request only when > !httpRequest? No special reason. I simply didn't change the old series of actions. I'm attaching a new patch that does the test for image first.

Comment on attachment 335737 [details] [diff] [review] unreal patch v3 r=bzbarsky

pushed to hg with revision f95e6897b536, marking fixed.

verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20080902041619 Minefield/3.1b1pre and the equivalent Windows XP build. I verified using the steps in Comment 0.

Could this be added to test-suite? Looks more reasonable as having a Litmus testcase for this issue.

I dont think this bug is fixed. I use NoScript and the latest nighly trunk version of Firefox and I am getting pages saying they are not secure and when looking at the Media tab under Page Info I see the following: resource://noscript_0.6457643962686347/icon32.png The same issue I am having with another extension which uses resource icons and that too is causing the page to show up as insecure.

Could you give an exact example of a secure page you see as broken, please?

https://virtualoffice.lss.ku.edu/NetStorage/ you will need to use the following NoScript settings to simulate my setup and see if the error can be reproduced: ser_pref("capability.policy.maonoscript.javascript.enabled", "allAccess"); user_pref("capability.policy.maonoscript.sites", "about: about:certerror about:config about:neterror about:plugins about:privatebrowsing about:sessionrestore chrome: https://virtualoffice.lss.ku.edu resource:"); user_pref("noscript.allowURLBarJS", false); user_pref("noscript.autoReload", false); user_pref("noscript.autoReload.allTabs", false); user_pref("noscript.autoReload.global", false); user_pref("noscript.blockCssScanners", true); user_pref("noscript.blockNSWB", true); user_pref("noscript.clearClick.exceptions", ""); user_pref("noscript.confirmUnblock", false); user_pref("noscript.consoleDump", 1); user_pref("noscript.consoleLog", true); user_pref("noscript.contentBlocker", true); user_pref("noscript.ctxMenu", false); user_pref("noscript.default", "chrome: resource: about:"); user_pref("noscript.docShellJSBlocking", 2); user_pref("noscript.filterXExceptions", "^http://([a-z]+)\\.google\\.(?:[a-z]{1,3}\\.)?[a-z]+/(?:search|custom|\\1)\\?\n^http://[a-z]+\\.wikipedia\\.org/wiki/[^\"<>\\?%]+$"); user_pref("noscript.firstRunRedirection", false); user_pref("noscript.forbidBookmarklets", true); user_pref("noscript.forbidChromeScripts", true); user_pref("noscript.forbidFrames", true); user_pref("noscript.forbidIFrames", true); user_pref("noscript.forbidIFramesContext", 0); user_pref("noscript.forbidImpliesUntrust", true); user_pref("noscript.forbidJarDocumentsExceptions", ""); user_pref("noscript.forbidMetaRefresh", true); user_pref("noscript.forbidXBL", 5); user_pref("noscript.gtemp", ""); user_pref("noscript.httpsForced", "virtualoffice.lss.ku.edu"); user_pref("noscript.httpsForcedExceptions", ""); user_pref("noscript.ignorePorts", false); user_pref("noscript.injectionCheck", 3); user_pref("noscript.intranetMaskRx", "^(1(27|0|92)\\.[\\d.]+)"); user_pref("noscript.lockPrivilegedUI", true); user_pref("noscript.notify", false); user_pref("noscript.notify.bottom", false); user_pref("noscript.notify.hidePermanent", false); user_pref("noscript.nselForce", false); user_pref("noscript.nselNever", true); user_pref("noscript.opacizeObject", 3); user_pref("noscript.options.tabSelectedIndexes", "1,0,1"); user_pref("noscript.policynames", ""); user_pref("noscript.secureCookies", true); user_pref("noscript.secureCookiesForced", ".virtualoffice.lss.ku.edu"); user_pref("noscript.showAllowPage", false); user_pref("noscript.showBlockedObjects", false); user_pref("noscript.showDistrust", false); user_pref("noscript.showDomain", true); user_pref("noscript.showGlobal", false); user_pref("noscript.showPermanent", false); user_pref("noscript.showTempToPerm", false); user_pref("noscript.showUntrusted", false); user_pref("noscript.showUntrustedPlaceholder", false); user_pref("noscript.temp", ""); user_pref("noscript.toolbarToggle", 0); user_pref("noscript.untrusted", ""); user_pref("noscript.version", "1.9.0.8");

Sorry, forgot to tell you to just cancel out of any login prompt and then NoScript should have the frames blocked from displaying click each one so they are activated, then refresh page. The error should occur then.

Has anyone been able to reproduce the issue using the above settings and url?

This is not caused by resource: but by data: as i can seen so far. It's unrelated to this bug nor to bug 477118. I filed a new bug 482245 about this issue.