We have too many bugs related to lock icon tracking, see bug 130949. IMHO the design of the initial implementation that relies on notifications to be sent was a bad design. While we have been able to use band-aids for a while, we have now reached a state (say bfcache) where the notifications are no longer guaranteed to be sent. See bug 358438. A new implementation is required that, IMHO, is directly integrated into the core loading mechanisms.

wonder if things have gotten better in the last year?

This should have been blocking for gran paradiso, we incorrectly indicate secure state for many situations where we shouldn't, that all of our competitors get right.

A related issue: when leaving an encrypted page, a dialog comes up warning you about it, but it's too late. The dialog comes up AFTER you've sent the insecure request and have received the response insecurely. And the dialog gives you no way to say "cancel". So, the dialog really ought to say "You're hosed." I'm sure there's a bug filed about that. IMO, it should be fixed when this bug is fixed. The new design should encompass both.

I was referring to Bug 62178 – there is no way "cancel" browsing to/from ssl'ed requests

I can't see us redoing the lock icon tracking mechanism at this point in the release. What *does* worry me is exactly which situations we transmit data insecurely with a false indicator that has caused us to declare that we need a new implementation immediately. Is there one in particular? I understand it could just be the sum of all. I'm -'ing this, adding to the wanted-next (which is blocking next, really), and pulling a few others into the conversation.

Jonath, care to comment here?

we really wouldn't/shouldn't make changes to this in a maintenance release so "blocking next" probably doesn't make sense. it really needs a beta to shake out any side effects, so that mean fx3 b5, or next major release...

wanted-next is the next major release. wanted1.9.0.x is maintenance release.

Consider that part of this issue, Bug 62178, is now 7 years old.

(In reply to comment #6) > Jonath, care to comment here? [I was emailed about bug 135007 earlier today, and my reply made sense here, where I haven't yet commented. Modified version included below.] Obviously, I think these bugs should be fixed. :) As Nelson and Kai will tell you, the various machinations in the interaction between NSS, PSM, and Docshell (to say nothing of what the browser then does with that information) make this no mean feat. I see this bug as the culmination of that frustration, and I am all for an approach here that the Security and Core groups can agree on. I think it's a step in the right direction because it tries to address a whole class of problems, rather than approaching it piecemeal. Rip-and-replace is a dangerous strategy, but so is piling on hacks and potentially (with each one) introducing regressions in behaviour. As for the sg: prioritization, I'm not stuck on it. They're variously sg:investigate/sg:low now, but spending energy to elevate it would be better spent, imo, getting boris or biesi excited about helping to fix it. In conjunction with Kai, who has made attempts at bugs like bug 62178 before, I think we could make progress. A patch for this bug would get attention, regardless of it's sg rating, in my opinion, and elevating it (sg:moderate? sg:high?) would not necessarily bring exciting new resources to the table - I wish we had a docshell+PSM hacker in a closet somewhere that we only brought out for the big jobs. If this was a UI problem, I'd get my hammer and bang it out myself, but the UI is relying on a broken system from Core, and a core hacker I am not. I will cc: Honza on the bug though, since he's been showing an interest in this type of work, and has been turning out some good patches too. Do we agree that re-architecture is our salvation here? We should think hard about it. If there are 5 or 7 or 24 known problems with our mixed content code, it might actually be more time efficient (if decidedly less elegant) to fix those and wrap them in sweet, sweet unit tests to prevent future regressions. If we do agree that rewrite is the only way to get right, what can I do to help?

I think we must start by fixing bug 135007 and bug 135011, so our mixed content implementation has a chance to become complete and be aware of images, too. Then, in order to correctly support the two modes "all insecure content shown" and "all insecure content blocked", we must fix bug 62178. This will allow us to be aware of the state of the current document (secure toplevel, insecure toplevel, broken toplevel, overriden toplevel) and make decisions about what content to show - based on policy, based on user overrides, etc. Once we've done that, we should (finally) have the infrastructure in place to track the security state based on docshell notifications - and stop relying on the late notifications. I suspect it will make sense to reimplement the lock icon tracking mechanism. But it will not help to simply rewrite the code in mozilla/security/manager. We need core backend support from docshell and networking etc., first!

(In reply to comment #11) > I think we must start by fixing bug 135007 and bug 135011, so our mixed content > implementation has a chance to become complete and be aware of images, too. So, we've fixed those now (thanks Kai and Honza!) Is this bug still proposing a distinct approach, or are we all just agreeing that we should fix bug 62178 here? If so, what is that approach, and is it still sg:moderate? If not, should we resolve this bug?

IMO, bug 62178 is a much-more-than-moderate vulnerability. I wonder if the work recently done for this bug explains all the new mixed content warnings I get now. For example, I get mixed content warnings for every page from my bank. For example, see https://sitekey.bankofamerica.com/sas/signonScreen.do?state=CA (Your mileage may differ with that page.)

I should have mentioned that that BofA page contains no http URLs for images or scripts, yet it always reports mixed content. :(

We can put another patches to the already a-lot-patched mixed content code to solve the problems we still have (and we still going to have, I believe) and fix somehow on top of it the "no-way-to-stop-non-ssl-from-ssl" issue, or rewrite it all from the base. I personally, maybe because I am perfectionist, vote for the letter way. Big effort, big design, but could solve issues for a long time forward.

This has been placed on our "Top Security Bugs" list. Please treat as a top priority.

Brandon, May I ask how, and by whom, it was placed on the top security bugs list? Has there been a change in the perception of the risks of the present implementation? Feel free to reply to email if you'd rather not post here.

Hi Nelson. I'll reply via email.

Based on some private emails I've received, I fear that my comment 17 has been interpreted as an objection to Brandon's directive to "treat as a top priority". Just to be clear, I have absolutely no objection, and indeed I welcome that decision. I was SO (pleasantly) surprised to read it that I reacted in shock. But please don't let my reaction get in the way of fixing this bug or any of the related bugs.

As I am the first person available that could work on this bug it would be great for me to know if there was found a new issue (not yet reported) I should be aware of or if the decision was based on existing issues. You can also reply by email. I am now working on regression automatic tests for mixed content btw based on Kai's manual tests from his site.

Hi, Honza. No, there haven't been any new issues associated with this bug. I'll forward you some email that explains the prioritization of this bug.

If there is any chance at making progress for this in Firefox 3.1 it probably needs to happen in the next few days to make the final beta. What are chances for that?

(In reply to comment #22) > If there is any chance at making progress for this in Firefox 3.1 it probably > needs to happen in the next few days to make the final beta. What are chances > for that? For Firefox 3.1 (Gecko 1.9.1) there is no chance. It is too late. This is huge task.

I encounter more and more bugs in this area. To fix them starts to be mess and I'm afraid fixing the current code might completely break the security mixed content detection and introduce more serious security issues. Assigning to me, soon I will start work on this full time.

Confirming that I'm intensively working on this bug at the moment.

cc'ing stephen horlander (theme) and alexander limi (firefox ux) so that they're in the loop. I don't think there's any action on their part, but understanding when we get signals about SSL state may be useful for them.

For now I'm releasing this bug. I believe we will have to sit and talk prior the work here can move forward. This is old, and there always was some mitigation of mixed-content issues like HSTS that lowered priority of this bug. Until it's concluded how the algorithm and UI/UX should look like, I don't want to keep blocking on this bug.

The UI for Mixed Content has been decided. See http://people.mozilla.com/~lco/ProjectSPF/Mixed_Content/Mixed%20Content%20Spec%20v3.pdf. I'm working on the implementation in bug 782654. Bug 62178 is fixed and landed. It adds about:config settings to block Mixed Active Content and Mixed Display Content. Bug 782654 is to create a blocking UI and set the about:config pref for Mixed Active Content to true. The lock icon states have also been decided and I've summarized them here: https://bugzilla.mozilla.org/show_bug.cgi?id=747090#c23

I will start working on this the next week since this code still gets back to me through its bugs. I really want to clean it up now also in light of the new nsMixedContentBlocker introduced in bug 62178.

I didn't start to work on this as much as I would like to. Here is at least a list of some elementary issues we have: 1. implementation of nsSecureBrowserUI: - it strongly depends on status notifications from docshell (loader?), mainly OnLocationChange and transport status that not always work well and is actually harder to use - OnLocationChange causes the sec UI to completely spill its internal state to track the new load that can also be a REFRESH load (back or forth navigation = bfcache) ; sounds simple, but it isn't! - we exchange state members of the security UI with the top level load channel of the document, to be exact: with security info object of the channel - we do this (btw) to let the channel store the top level document security state (secured/mixed/unsecured) info in http cache what is needed for bfcache and session restore as well ; not really a good design (there are known serious bugs, TODO: list them here) - the state is actually stored on two places (the UI and the secinfo of the channel) and has to sync when location changes (dirty way of programming for me) - thanks this way of implementation there were also bugs that loads coming from e.g. onmouseover styling handlers changing an image of a button were counted to in-progress loads of forward navigation page (after a link click, e.g.) when OnLocationChange has already been notified to the sec UI We have to change how the state is held in memory as well as how it gets persisted. 2. behavior: - the UI tries to determine whether the content of the page really displays or actively uses some content that has been received unsecured (only then the state is mixed); this is different from what bug 62178 does, the mixed content blocker doesn't allow load before it happens, (which is good), but the definition of "mixed content" then has changed for us - the previously described behavior was engaged because some pages were doing some (probably cross domain tracking?) requests for e.g. images that didn't return any content but produced a mixed content state ; there were bugs where we broke the UI sec state of some pages when we didn't concern this correctly (TODO: find those bug #s) - for images there are bugs in imglib that some notification contracts (that sec UI deps on) are broken by image proxies that mainly open these issues ; I personally would like not to be dependent on those bugs The behavior (=what is exactly "mixed content"?) is something I'd like to talk about first with all involved people here. 3. the way it notifies the state: - the security state notification is made through call to OnSecurityChange (made by sec UI) that arrives at the browser UI listening to it - this call has to be duplicated - we have to do this duplication, because consumers not always get OnSecurityChange and OnLocationChange in the correct order (location change first), e.g. when switching tabs (TODO: list bugs here) - this is not caused by the UI it self, but by the way observers non-deterministically work Here I don't have idea what to do exactly, but consumers should load the state from the sec UI [also] only when location is changed, since we know the top level state at that moment, and notify when the state lowers to mixed only * This means that sec UI shouldn't be based on notifications from docshell but on something a bit more lower level. I know it would be nice to use generic approach here by just "putting" sec UI into the system of listeners and leave this implementation fully in PSM, but I think it would be appropriate to have some immediate support from lower levels (docshell and/or netwerk) as well. I mean here to add some sec state detection support directly into those modules. * OnLocationChange is called when the top level channel starts loading the content in its associated doc shell, it is actually indirectly coming from OnStartRequest called by the channel ; it means that e.g. redirects will not cause OnLocationChange until resolved