If the user has added a security exception, the user will get a green icon and a secure connection text, unless expand to see the "You have added a security exception for this site." (identity.identified.verified_by_you)". I'm not sure but maybe we should change it. But if we think that is safe with added exception certificate, we can maintain this situation.

Tanvi - can you weigh in on this? Not sure how the user does it, but guessing we want to continue to reflect it as insecure...

We can show a tip text and keep it is green (secure connection), if this situation is usually safe. Like the https://pike.github.io/firefox-l10n-tests/?file=security%2Fmixed-content-blocking.png&locale=en-US and https://pike.github.io/firefox-l10n-tests/?file=security%2Fmixed-content-blocking-expanded.png&locale=en-US.

Some examples of why a user would have to add a security exception: * domain mismatch * expired cert * self signed cert (with potential to be MITM'd) We should downgrade to grey lock with yellow triangle in the url bar and in the control center (similar to https://people.mozilla.org/~tvyas/mixeddisplay.html with different text). We should say "Connection is Not Secure". On the platform side, this means setting the state to STATE_IS_BROKEN instead of STATE_IS_SECURE. This will be a good reminder to developers that they need to fix their certificate before going to prod. And a good indication to users that they stored a permanent exception to this site at some point (could have been years ago) and shouldn't be 100% confident in its security. (Note that the default is to permanently store these exceptions.) Dan makes a point that this might cause some backlash - I’ve told Firefox I trust this site so why am I getting degraded UI? Moreover, how will the user know why they are getting this iconography? If a page with a self signed cert introduces mixed content, the user will assume the degraded UI is because of the cert and not click the control center to further drill down into the problem. I want to avoid adding more iconography wherever possible since I think it will confuse more users than it benefits. The grey lock with the yellow triangle sets the “caution” tone I want to set for the everyday user. And the more advanced users know how to learn more. Chrome’s actually degrades self-signed certs to a crossed out lock, which is more alarming than this proposal. Dan also brought up a good point about user installed trusted root certificates. Adding a certificate exception takes 3 clicks. But adding a new trusted root takes 2 clicks. New roots would likely be added by an employees organization (before they even receive their computer) and in some cases an employer might give employees steps to follow to add them. But what is to prevent an attacker from trying to get a user to add one with 2 clicks? The dialog the user has to click through is much less scary than the one for a certificate exception. But since it is heavily relied on by corporations, degrading the UI for an added root is a no-go. I hear Chrome uses different (not degraded just different) UI in this case and I’m inquiring to learn more. We can file a separate bug for that experience.

After thinking about this more, I'm convinced we should show degrade security UI for pages that required the cert override. What I'm debating between is which icon to use: 1) Lock with yellow warning triangle over it. 2) Lock with strike through. Chrome uses a lock with a strike through. For general users, the lock with the strike through is a good option since exceptions are permanent by default, and this is a good more noticable reminder that something is off with the page. The lock with the strike through is very rarely used right now (Mixed Active Content blocking disabled) and will be used for an equally if not more alarming issue in the future (password field over HTTP). The lock with the yellow triangle is not great for general users since they see it frequently for Mixed Display Content. But for developers, I prefer to use the lock with the yellow triangle. Developers may be using a self-signed cert and hence may just want to ignore this warning during development. The lock with the strikethrough would be too alarming in this case and I don't want developers to become habituated to it. I want them alarmed when they see it in FF 44 when a password is over HTTP. Using two different icons for two different populations is too confusing though. Using two different icons depending on the cert error type (ex: self-signed vs something else) is also too confusing. I would also prefer not to add yet another icon. So in the end, we need to settle on one of these. Bram is considering these options.

I think we should use the lock with the strike for both to be consistent if the user did an explicit override action to be able to view the page. That keeps it similar to overriding mixed active content blocking. The yellow one is usually only if something isn't done by the user.

Does the frontend know when a page has gone through a cert override? If yes, it will be fairly simple to check the cert override flag per page and change the security UI accordingly. If no, where is this information stored in the platform? Thanks!

I don't know off-hand if we have a flag for this. Would be great if someone could investigate from the frontend side, Paolo or Brian maybe?

Looks like the frontend currently uses the cert override service to show that text: https://dxr.mozilla.org/mozilla-central/rev/2b333a1d94e805a59c619ee41a6dec7fdcce505d/browser/base/content/browser.js#7218 (I imagine the same check could be made to show a different icon)

this._sslStatus seems to give some useful data (like isNotValidAtThisTime which seems to be true when loading https://expired.badssl.com/ and adding an exception). There's a bunch of other properties on there, including this._sslStatus.serverCert which has even more. https://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#7129

Also just saw Comment 8 which seems to answer the original question

Hi, I'm the new intern and I'll be working on this bug. The frontend work for this bug seems straightforward. Can someone confirm which icon we want to use? comment 5 suggests the lock with the strike-through, shall I go ahead with that? As for the platform side, Tanvi says in comment 3 that we should probably use STATE_IS_BROKEN. I spoke to her today in person though and she suggested maybe adding a new state altogether for the user exception case. After spending some more time on DXR though, I'm not sure this is the greatest idea, since when an exception is added, we want consumers of the state info to see the site as secure, right? If this bug is just about degrading the UI to give the user a visual reminder that they added an exception, I want to point out that it's possible to do that without touching platform (the cert override service can be used from frontend). David, what do you think? Assuming we want to add the new state on the platform side, can you help me figure out where in the flow this new state should be set? After going through nsSecureBrowserUIImpl.cpp for a while, I tentatively think this might be ok: a) Add the new state in the idl: https://dxr.mozilla.org/mozilla-central/rev/584870f1cbc5d060a57e147ce249f736956e2b62/uriloader/base/nsIWebProgressListener.idl#148 b) Add a new lockIconState value representing the user-overriden state here: https://dxr.mozilla.org/mozilla-central/rev/584870f1cbc5d060a57e147ce249f736956e2b62/security/manager/ssl/nsSecureBrowserUIImpl.h#58 c) Add a new global for this in nsSecureBrowserImpl.cpp (mCertUserOverridden? can be bikeshedded later) along with mSubRequestsBrokenSecurity/mSubRequestsNoSecurity. Consult the cert override service and set the variable in OnStateChange - https://dxr.mozilla.org/mozilla-central/rev/584870f1cbc5d060a57e147ce249f736956e2b62/security/manager/ssl/nsSecureBrowserUIImpl.cpp#466. d) Use that to set mNotifiedSecurityState in UpdateSecurityState - https://dxr.mozilla.org/mozilla-central/rev/584870f1cbc5d060a57e147ce249f736956e2b62/security/manager/ssl/nsSecureBrowserUIImpl.cpp#1122. e) Use that to set the state - https://dxr.mozilla.org/mozilla-central/rev/584870f1cbc5d060a57e147ce249f736956e2b62/security/manager/ssl/nsSecureBrowserUIImpl.cpp#215. f) Update other consumers of the lockIconState and the security state wherever needed. That should be enough to correctly broadcast the state via the onSecurityChange notification, I think. Anyway, I need a sanity check on whether I'm looking at this the right way. Should I be doing the cert override check at a lower level? I feel like I might have spent too much time speculating on this and should have just asked in the beginning (especially if we don't want to add a new state security flag after all). Oh well. This was a long comment - thanks in advance! PS: I'd like to give bug 832834 a special mention here :P

Hi Nihanth, Solid investigation so far. I think you're right that neither STATE_IS_BROKEN nor an entirely new state is the safest way to go. We could potentially treat this like the weak crypto case, where we have a base state of STATE_IS_BROKEN and then we set an additional flag STATE_USES_WEAK_CRYPTO, except instead of the base state being STATE_IS_BROKEN, we could use STATE_IS_SECURE. This is where the platform sets the state/flags: https://dxr.mozilla.org/mozilla-central/rev/584870f1cbc5d060a57e147ce249f736956e2b62/security/manager/ssl/nsNSSCallbacks.cpp#1239 (note that the STATE_SECURE_HIGH flag is superfluous - see bug 1095602 on removing it). Long story short, I think your plan is good, but I would add in an initial step of determining if the connection has a certificate override on it in nsNSSCallbacks.cpp and setting the new flag if so (but again, keeping the base state at STATE_IS_SECURE). Then, the frontend can do whatever it wants with that information.

(In reply to Nihanth Subramanya [:nhnt11] from comment #11) > comment 5 suggests the lock with the > strike-through, shall I go ahead with that? Yes. Let’s use the lock with strike-through icon.

(In reply to Bram Pitoyo [:bram] from comment #13) > (In reply to Nihanth Subramanya [:nhnt11] from comment #11) > > comment 5 suggests the lock with the > > strike-through, shall I go ahead with that? > > Yes. Let’s use the lock with strike-through icon. Talked to Bram and Nihanth on irc and decided to use the lock with the yellow triangle. Since developers may be using self-signed certs and going through cert overrides a lot, we don't want them to be habituated to the lock with the strikethrough. Especially since we just landed a change that shows that icon for login forms on HTTP pages (FF 47 Dev edition). The lock with the yellow triangle is better than what we have now (a green triangle) and we can always make it more harsh in the future if we feel that the lock with the yellow triangle is no longer alarming enough.

This adds a STATE_USER_OVERRIDDEN_CERT flag to nsIWebProgressListener, and sets it on the security state in nsNSSCallbacks.cpp. David, I'm requesting f? from you since you've been helping me on this so far, feel free to redirect to someone else if you're not the right reviewer for this. It also attempts to correctly broadcast it from nsSecureBrowserUIImpl.cpp. I say "attempts" because while it works (browser.js sees the flag set correctly), I am unsure about a few things: a) I haven't messed with the lockIconState stuff, seems like that's a bad idea after looking at it again. Instead I just set a global (mUserOverriddenCert). b) I'm setting the global directly in UpdateSecurityState() - do you think I should use two variables instead (New and Notified versions, like the other stuff)? Or set it somewhere else altogether? Somehow something about this feels incomplete, please let me know if I missed something. Also by the way, are/will there be any potential consumers of the new flag on the platform side? Or are we just future-proofing? Frontend patch is on its way.

Comment on attachment 8715589 [details] [diff] [review] Platform patch v1 Review of attachment 8715589 [details] [diff] [review]: ----------------------------------------------------------------- This looks good so far. I have a couple of comments for things to check out. ::: security/manager/ssl/nsNSSCallbacks.cpp @@ +1246,5 @@ > + } > + > + bool certIsUserOverridden = false; > + nsCOMPtr<nsICertOverrideService> overrideService = > + do_GetService(NS_CERTOVERRIDE_CONTRACTID); I think there's an effort to stop calling the component manager from off the main thread, so this might not be something we want to do here. I'd ask Nathan Froyd [:froydnj] about it (see bug 1239421). @@ +1247,5 @@ > + > + bool certIsUserOverridden = false; > + nsCOMPtr<nsICertOverrideService> overrideService = > + do_GetService(NS_CERTOVERRIDE_CONTRACTID); > + nit: really only need one blank line here @@ +1256,5 @@ > + bool isTemporaryOverride; // Unused. > + const nsACString& hostString(infoObject->GetHostName()); > + const int32_t& port(infoObject->GetPort()); > + nsCOMPtr<nsIX509Cert> cert; > + status->GetServerCert(getter_AddRefs(cert)); I think you'll want to get the server cert after the logic around line 1312. ::: security/manager/ssl/nsSecureBrowserUIImpl.cpp @@ +800,5 @@ > { > f -= nsIWebProgressListener::STATE_SECURE_HIGH; > info.AppendLiteral("SECURE_HIGH "); > } > + if (f & nsIWebProgressListener::STATE_USER_OVERRIDDEN_CERT) FYI, this whole block is of rather suspect value. See bug 1127158. @@ +1143,5 @@ > if (mNewToplevelSecurityState & STATE_IS_BROKEN) { > newSecurityState = lis_broken_security; > } > > + mUserOverriddenCert = (mNewToplevelSecurityState & STATE_USER_OVERRIDDEN_CERT); nit: watch out for lines >80 chars ::: uriloader/base/nsIWebProgressListener.idl @@ +16,5 @@ > * requests in the context of a nsIWebProgress instance as well as any child > * nsIWebProgress instances. nsIWebProgress.idl describes the parent-child > * relationship of nsIWebProgress instances. > */ > +[scriptable, uuid(4290ae82-e559-49e9-98cf-a0b073afb5f7)] I think revving uuids isn't necessary anymore: https://groups.google.com/forum/#!topic/mozilla.dev.platform/HE1_qZhPj1I

(In reply to David Keeler [:keeler] (use needinfo?) from comment #16) > ::: security/manager/ssl/nsNSSCallbacks.cpp > @@ +1246,5 @@ > > + } > > + > > + bool certIsUserOverridden = false; > > + nsCOMPtr<nsICertOverrideService> overrideService = > > + do_GetService(NS_CERTOVERRIDE_CONTRACTID); > > I think there's an effort to stop calling the component manager from off the > main thread, so this might not be something we want to do here. I'd ask > Nathan Froyd [:froydnj] about it (see bug 1239421). That bug seems to be about creating instances directly. Is there precedent that I can refer to for whatever I should use instead of do_GetService?

Patch for frontend. I've introduced a new value for the connection attribute "secure-user-override", maybe this should be "insecure-user-override"? What do y'all think? Platform treats the exception case as "secure", but in the UI we are now saying "insecure", so I'm not sure what to use. Marco, Bugzilla suggested you as a reviewer and that didn't seem insane to me, but feel free to redirect it to the right person if that's not you.

Screenshots for good measure. Sorry for the low resolution, took them on my external screen.

Comment on attachment 8716136 [details] [diff] [review] Frontend patch v1 Review of attachment 8716136 [details] [diff] [review]: ----------------------------------------------------------------- I'm forwarding to Paolo that has a better understanding of the security panel

The patch looks quite good. I'd like to spend some time thinking about the interaction with the other security states, but they're probably going to be correct in this version. In the meantime, you can start working on a browser-chrome test to exercise this state and verify that we display the right icon, similar to the Mixed Content Blocker tests or the Insecure Login Form ones.

(In reply to Nihanth Subramanya [:nhnt11] from comment #17) > (In reply to David Keeler [:keeler] (use needinfo?) from comment #16) > > ::: security/manager/ssl/nsNSSCallbacks.cpp > > @@ +1246,5 @@ > > > + } > > > + > > > + bool certIsUserOverridden = false; > > > + nsCOMPtr<nsICertOverrideService> overrideService = > > > + do_GetService(NS_CERTOVERRIDE_CONTRACTID); > > > > I think there's an effort to stop calling the component manager from off the > > main thread, so this might not be something we want to do here. I'd ask > > Nathan Froyd [:froydnj] about it (see bug 1239421). > > That bug seems to be about creating instances directly. Is there precedent > that I can refer to for whatever I should use instead of do_GetService? We don't have a great story yet for services. Even mozilla/Services.h winds up calling into the component manager. So I'd say go ahead with what you have.

I piggybacked on the existing browser_addCertException test, hope that's okay. The test is pretty much identical to the insecure login forms test, i.e. it just checks the icons.

Added a couple of comments.

Addressed your review comments. Left the do_GetService as it is (refer comment 22). I need to check whether there are tests I need to write/update for this (I assume there are). I'll do a try push once I figure those out.

Ninanth or David, is there a webpage I can use to test this functionality interactively?

Comment on attachment 8716540 [details] [diff] [review] Platform patch v2 Review of attachment 8716540 [details] [diff] [review]: ----------------------------------------------------------------- ::: uriloader/base/nsIWebProgressListener.idl @@ +268,4 @@ > */ > const unsigned long STATE_USES_SSL_3 = 0x01000000; > const unsigned long STATE_USES_WEAK_CRYPTO = 0x02000000; > + const unsigned long STATE_USER_OVERRIDDEN_CERT = 0x04000000; I'd like to see more consistent naming throughout the front-end and the back-end. At present, sometimes CERT is placed before, sometime it's after the rest. To avoid the confusion between USER and USES in the lines above, I'd place CERT before in all places. STATE_CERT_USER_OVERRIDDEN mCertUserOverridden _isCertUserOverridden classList.add("certUserOverridden") connection = "secure-cert-user-overridden" Or you could use OVERRIDE instead of OVERRIDDEN all over the place. Maybe the platform has conventions around this, for front-end it's the same, just make it consistent with the platform naming. For the front-end classes, you could use "userOverridden" and "secure-user-overridden" if you think other types of override that are not certificates may result in the same security indication.

(In reply to YF (Yang) from comment #27) > https://badssl.com/ Thanks :-)

Comment on attachment 8716536 [details] [diff] [review] Frontend tests v1.1 Review of attachment 8716536 [details] [diff] [review]: ----------------------------------------------------------------- These look good, but I think you should add code to close the panel. ::: browser/base/content/test/general/browser_addCertException.js @@ +70,1 @@ > gBrowser.selectedBrowser.removeEventListener("load", this, true); Remove the event listener first. You may need to make checkControlPanelIcons asynchronous to wait for the panel to be closed, in which case you can just execute the rest of the code below in a callback or a Promise "then" function.

Comment on attachment 8716136 [details] [diff] [review] Frontend patch v1 Review of attachment 8716136 [details] [diff] [review]: ----------------------------------------------------------------- Still a few concerns about naming, I wouldn't ask if this wasn't actually important to keep this area of the code manageable and understandable as we add more states to it. ::: browser/base/content/browser.js @@ +6884,3 @@ > tooltip = gNavigatorBundle.getString("identity.identified.verified_by_you"); > } > + else { "} else {" on the same line ::: browser/components/controlcenter/content/panel.inc.xul @@ +24,5 @@ > <vbox id="identity-popup-security-content" flex="1"> > <label observes="identity-popup-content-host"/> > <description class="identity-popup-connection-not-secure" > value="&identity.connectionNotSecure;" > + when-connection="not-secure secure-user-override"/> Hm, actually it seems we're not clear on whether we consider the connection secure or not... How does the platform call this state? If it says "secure" somewhere in the flags because we have a certificate, then the "secure" prefix here is fine. Otherwise, this is more of a connection="not-secure-cert-user-overridden" state. Or if there is still ambiguity, we can just call this connection="cert-user-overridden". @@ +53,5 @@ > > <!-- Tracking Protection Section --> > <hbox id="tracking-protection-container" > class="identity-popup-section" > when-connection="not-secure secure secure-ev file"> Seems likely we'd like to show the TP section in pages with security overrides too.

Comment on attachment 8716540 [details] [diff] [review] Platform patch v2 Review of attachment 8716540 [details] [diff] [review]: ----------------------------------------------------------------- I'm fairly sure this is correct. It would be good to test this thoroughly - e.g. when the override is top-level vs. from a sub-resource/iframe, etc. r=me with comments addressed. ::: security/manager/ssl/nsNSSCallbacks.cpp @@ +7,5 @@ > #include "nsNSSCallbacks.h" > #include "pkix/pkixtypes.h" > #include "mozilla/Telemetry.h" > #include "mozilla/TimeStamp.h" > +#include "nsICertOverrideService.h" Maybe take this opportunity to sort these #includes? (With nsNSSCallbacks.h first, and then a blank line, and then the rest of them.) @@ +1267,5 @@ > + bool haveOverride; > + uint32_t overrideBits = 0; // Unused. > + bool isTemporaryOverride; // Unused. > + const nsACString& hostString(infoObject->GetHostName()); > + const int32_t& port(infoObject->GetPort()); nit: this one doesn't need to be a reference, but no big deal @@ +1281,5 @@ > + } > + } > + > + uint32_t state; > + if (usesWeakCipher || renegotiationUnsafe) { I think this is an artifact of the diff algorithm, but this patch makes it look like much of this other state setting code has moved around. Can you re-work this so just the nsICertOverrideService/certIsUserOverridden/nsIWebProgressListener::STATE_USER_OVERRIDDEN_CERT code is after the nsSSLStatus code? (Since there aren't any early returns, it shouldn't matter either way, and it's easier to manage code when each patch modifies only what's necessary to make a change.)

Carrying forward the r+

(In reply to :Paolo Amadini from comment #31) > Comment on attachment 8716136 [details] [diff] [review] > Frontend patch v1 > > Hm, actually it seems we're not clear on whether we consider the connection > secure or not... > > How does the platform call this state? If it says "secure" somewhere in the > flags because we have a certificate, then the "secure" prefix here is fine. Platform treats this as a "secure" state, so I'm keeping the secure prefix. (In reply to :Paolo Amadini from comment #30) > Comment on attachment 8716536 [details] [diff] [review] > Frontend tests v1.1 > > You may need to make checkControlPanelIcons asynchronous to wait for the > panel to be closed, in which case you can just execute the rest of the code > below in a callback or a Promise "then" function. I just set hidden to true, this is what the insecure login forms test does also: https://dxr.mozilla.org/mozilla-central/rev/584870f1cbc5d060a57e147ce249f736956e2b62/browser/base/content/test/general/browser_insecureLoginForms.js#75 (In reply to :Paolo Amadini from comment #28) > Comment on attachment 8716540 [details] [diff] [review] > Platform patch v2 > > I'd like to see more consistent naming throughout the front-end and the > back-end. At present, sometimes CERT is placed before, sometime it's after > the rest. To avoid the confusion between USER and USES in the lines above, > I'd place CERT before in all places. > > STATE_CERT_USER_OVERRIDDEN > mCertUserOverridden > _isCertUserOverridden > classList.add("certUserOverridden") > connection = "secure-cert-user-overridden" I've used these names, they make sense to me.

Comment on attachment 8717650 [details] [diff] [review] Frontend patch v2 Thanks!

Comment on attachment 8717651 [details] [diff] [review] Frontend tests v2 (In reply to Nihanth Subramanya [:nhnt11] from comment #36) > I just set hidden to true, this is what the insecure login forms test does > also: It's fine for now if it just works. At some point we'll need to make all of these operations asynchronous because the panel does not actually close immediately, but we can do that work for all the tests at the same time.

Checks for the STATE_CERT_USER_OVERRIDDEN flag in cert override tests.

Comment on attachment 8718161 [details] [diff] [review] Platform tests Review of attachment 8718161 [details] [diff] [review]: ----------------------------------------------------------------- Forgive the drive by review... ::: security/manager/ssl/tests/unit/head_psm.js @@ +663,5 @@ > add_connection_test(aHost, aExpectedError, null, > add_cert_override.bind(this, aHost, aExpectedBits)); > + add_connection_test(aHost, PRErrorCodeSuccess, null, aSecurityInfo => { > + Assert.ok(aSecurityInfo.securityState & > + Ci.nsIWebProgressListener.STATE_CERT_USER_OVERRIDDEN); If you feel like it, it would be nice to include an expected result message here (makes reading test logs easier, etc).

(In reply to :Cykesiopka from comment #40) > Comment on attachment 8718161 [details] [diff] [review] > Platform tests > > Review of attachment 8718161 [details] [diff] [review]: > ----------------------------------------------------------------- > [...] it would be nice to include an expected result message here [...] Done.

Comment on attachment 8718242 [details] [diff] [review] Platform tests v1.1 Review of attachment 8718242 [details] [diff] [review]: ----------------------------------------------------------------- Great

Try push revealed plenty of bustage - https://treeherder.mozilla.org/#/jobs?repo=try&revision=c7a66d9d33cd. Can't say I'm surprised. Investigating.

OK, I've fixed the bustages. Here's another try push - I quickly went through all the oranges and none of them seem to be related to this bug. https://treeherder.mozilla.org/#/jobs?repo=try&revision=127f526ef097

Ensure the cert override service is initialized on the main thread, as suggested by David on IRC. Carrying forward the r+.

Attached the wrong patch, oops.

Rebased

Marking this checkin-needed. Try push is in comment 45.

https://hg.mozilla.org/integration/fx-team/rev/4dd771bf8c98 https://hg.mozilla.org/integration/fx-team/rev/9e3ebc116454 https://hg.mozilla.org/integration/fx-team/rev/aa26b10363b1 https://hg.mozilla.org/integration/fx-team/rev/8661318f11a9

Hey Nihanth, for the future if you're not using mozreview which keeps track of the order of your patches, please include "Part N" in your attachment names so they land in the correct order. You were also missing some reviewers in the attachment so I tried to fill them in. Thanks.

https://hg.mozilla.org/mozilla-central/rev/4dd771bf8c98 https://hg.mozilla.org/mozilla-central/rev/9e3ebc116454 https://hg.mozilla.org/mozilla-central/rev/aa26b10363b1 https://hg.mozilla.org/mozilla-central/rev/8661318f11a9

http://i.imgur.com/IgD7mo1.png - both grey and yellow icons What do you think? Is it expected?

(In reply to Paul Silaghi, QA [:pauly] from comment #53) > http://i.imgur.com/IgD7mo1.png - both grey and yellow icons > What do you think? Is it expected? Nice catch Paul! In the mixed active content blocked + certificate override case, we should show the grey lock with the yellow triangle in the location bar. * mixed active content blocked by itself - green lock with the grey warning triangle. * certificate override case by itself - grey lock with the yellow warning triangle. * mixed active content blocked + certificate override - the policy is to show the most degraded security indicator, so in this case, its the grey lock with yellow warning triangle.

Marking as verified on 47.0a1 (2016-03-02) Win 7, considering the follow-up bug.