Closed Bug 405145 Opened 16 years ago Closed 16 years ago

Mixed SSL/non-SSL content reporting inconsistent between standard and EV Certs


(Core Graveyard :: Security: UI, defect, P2)



(Not tracked)



(Reporter: wgianopoulos, Assigned: wgianopoulos)




(1 file)

If you visit an https site that presents a non-EV Certificate that has mixed SSL and non-SSL content on the page, Larry gives the same indications as if the site had no certificate and was not encrypted at all.

If you do the same on a website that presents an EV Certificate, you get the Identity Verified green checkmark plus the text stating that the site is encrypted to prevent eavesdropping.  This behavior is not terribly consistent.

In my opinion, both results are incorrect.  I don't think the mix of SSL and non-SSL content should really change the display relative to the site identity.  After all, that really only refers to the identity of the site mentioned in the URL.  As far as I know, in the it is all SSL case, that is all that is checked for, that it is all SSL.  It matters not if all the content comes from the same server, uses the same certificate or comes from the same domain, just hat it is all encrypted.  So, as ling as the Identity indicator does not say all the content comes from these people as identified by this certificate, as far as the Identity part goes whether or on it is all encrypted is immaterial.

As far as the mixed content goes, I thing the text at the bottom about encryption needs to be altered to say something to the effect that not all the content is encrypted.
Flags: blocking-firefox3?
Assignee: nobody → johnath
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P2
In the case where the site is mixing SSL content with non-SSL content, whether
the SSL is EV or not, the identity really should be presented as unconfirmed.

Asserting the identity of the page implies that what you are seeing is what the
site intended you to see, but if there are non-SSL parts to that content, then
they can be altered undetectably, so we can no longer say "yes, this is paypal,
and you can make decisions about this site based on your trust relationship
with paypal" since parts of it could be from non-paypal sources.

Now, if SSL-verified links to other sites using SSL connections, if
they have image hosting off-site for instance, we can still confidently assert
paypal's identity as the top level site, since we can confirm the identity and
integrity of the base page, and of all the content.  But in any event, there
shouldn't be a difference in this behaviour between EV and SSL.  Either we can
confirm the content or we can't.

There are several bugs already on breakages in our mixed content detection, but
this is the first I've heard of EV and SSL getting different treatments. 
Moving to PSM, since (unless there's a bug there that I'm not aware of) Firefox
just reflects the security state as sent from PSM.
Assignee: johnath → kengert
Component: General → Security: UI
Flags: blocking-firefox3+
Product: Firefox → Core
QA Contact: general → ui
OK.  Unfortunately the only testcase I have for this is not with a Versign certificate.

The site is which uses a Comodo EV certificate, so in order to see the issue you need to add the following to nsIdentityChecking.cpp:

    "Comodo EV OID",
    "CN=UTN-USERFirst-Hardware,OU=,O=The USERTRUST Network,L=Salt Lake City,ST=UT,C=US",
    "CN=UTN-USERFirst-Hardware,OU=,O=The USERTRUST Network,L=Salt Lake City,ST=UT,C=US",
(In reply to comment #1)

> Moving to PSM, since (unless there's a bug there that I'm not aware of) Firefox
> just reflects the security state as sent from PSM.

Well, I am beginning to think this has to be a Firefox bug, as the padlock in the status bar as well as pageinfo show the correct information.  It is just Larry that does not.
Although I suppose the code here:

could be modified to only set STATE_IDENTITY_EV_TOPLEVEL if STATE_IS_SECURE is already set.
Attached patch Patch v1Splinter Review
This resolves the issue for me.
Assignee: kengert → wgianopoulos
Attachment #289973 - Flags: review?(rrelyea)
This alters the EV cert case to work identically to the way the non-EV cert case currently does.
Comment on attachment 289973 [details] [diff] [review]
Patch v1

I think this makes a lot of sense. Thanks.
Attachment #289973 - Flags: review?(rrelyea)
Attachment #289973 - Flags: review+
Attachment #289973 - Flags: approval1.9?
Attachment #289973 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Attachment #289973 - Attachment description: like this? → Patch v1
Fixed as part of check in for bug 402574.

Bill, thanks a lot for your help!
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.