Closed Bug 1126413 Opened 7 years ago Closed 7 years ago

Firefox should show better indicators for RC4 sites

Categories

(Firefox :: Address Bar, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 38
Tracking Status
firefox38 --- fixed

People

(Reporter: ekr, Assigned: emk)

References

(Depends on 1 open bug)

Details

Attachments

(2 files)

Test case:
https://whatwg.org/

This presently shows a triangle, most likely because it is RC4.
However, this means you also get no site identity information,
which is sad.
The security UI can not access the information without this change.
Attachment #8556051 - Flags: review?(dkeeler)
Attached patch UI changesSplinter Review
Attachment #8556052 - Flags: review?(dolske)
Comment on attachment 8556051 [details] [diff] [review]
Expose nsISSLStatus for broken secure pages

Review of attachment 8556051 [details] [diff] [review]:
-----------------------------------------------------------------

I believe this is correct. Be aware that for connections that are broken as in couldn't we even complete the handshake and the user can't add an override, there is no sslStatus object (but that's not what broken represents in this case, since we've connected regardless of some brokenness and do have an sslStatus).
Attachment #8556051 - Flags: review?(dkeeler) → review+
Does this regress bug 420246 (which added the nulling)?

I'm a little wary of there being code that's relying on not getting an sslStatus when the connection is broken (ie, isn't otherwise checking if the info it gets is actually valid)?
After applying this patch, I couldn't reproduce anything like the behavior described in bug 420246. I agree the situation regarding the network layer indicating security state isn't optimal, but in the case of a completely broken connection, Necko terminates the connection and shows one of those grey error pages, so I don't think there's too much danger there (unless I'm misunderstanding your concern).
(In reply to Justin Dolske [:Dolske] from comment #4)
> Does this regress bug 420246 (which added the nulling)?
> 
> I'm a little wary of there being code that's relying on not getting an
> sslStatus when the connection is broken (ie, isn't otherwise checking if the
> info it gets is actually valid)?

Mixed content pages already have STATE_IS_BROKEN and already returns SSLInfo because mixed content pages are classified as lis_mixed_security inside nsSecureBrowserUIImpl. So if this change causes any problems like bug 420246, it must have already happened on mixed content pages.

Also I couldn't reproduce such a problem either.
Comment on attachment 8556052 [details] [diff] [review]
UI changes

Review of attachment 8556052 [details] [diff] [review]:
-----------------------------------------------------------------

Ok.
Attachment #8556052 - Flags: review?(dolske) → review+
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
https://hg.mozilla.org/mozilla-central/rev/f6594816e867
https://hg.mozilla.org/mozilla-central/rev/8d1bb05dffa7
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
Duplicate of this bug: 1132869
Depends on: 1134086
You need to log in before you can comment on or make changes to this bug.