Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 6 users
Status: Fixed
Owner:
Closed: May 2010
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug
M-6

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment
Make mixed-content indicators correct
Project Member Reported by sky@chromium.org, Apr 28 2010 Back to list
For https with mixed content we show the https in the omnibox in gray. 
Visually this makes me think I shouldn't care (gray is usually used to de-
emphasize something). When I showed it to Glen he wasn't thrilled about it 
either and though we should be using red in this situation.
 
Summary: Is grey "https" for mixed content the best thing? (was: NULL)
I don't think any change should be made here unless we want to agree security-team-
wide that mixed content is just as bad as broken-https.  If not, then we shouldn't 
use a red scheme (and we definitely shouldn't share half our indicator state with 
broken-https but not the other half -- the meaning of red scheme with grey lock 
versus red scheme with skull is not really obvious).

IMO the current situation is exactly what we want -- https is present, but you're not 
terribly secure, so you shouldn't see it as a strong signal.  "makes me think I 
shouldn't care" is a _perfect_ reaction.  It's not misleading at all.

This was also OKed by the Big Meeting Of Security And UI Folks several weeks back, 
although it wasn't the main focus.
Labels: -Area-Undefined Area-UI Feature-Omnibox
Note: In a separate email, cevans noted to me that we seem to be very aggressive about 
displaying the "broken https" state for mixed content warnings on trunk.  This isn't a 
change I intended.  So we may need to both "decide what the UI should look like" and 
"figure out what the UI is actually doing currently".  It might be that this aggressive 
UI is a result of some of wtc's recent changes.
Mixed content is subdivided into two categories:
1) Mixed content warnings: e.g. <img> loads
2) Mixed content errors: e.g. <script> loads

Mixed content warnings are not as serious as broken-https whereas mixed content 
errors are definitely as serious; possibly worse.

For me, trunk is showing the red https warning for 1) and 2). I like mostly like 
this.

It certainly should do the red https for 2) but doing so for 1) may well be overkill.

Comment 5 by jsc...@chromium.org, Apr 28 2010
Agreed with Chris. The danger of a compromised SSL connection is essentially the same 
as unauthenticated active (eg. script, flash) mixed content. For unauthenticated 
scripts in mixed content we should be red, but static content is much less of a risk 
and we shouldn't give them equivalent indicators.

Comment 6 by meta...@gmail.com, Apr 29 2010
Given that the padlock/skull is shown, the https: is completely redundant; see #42829.
I don't know how much we can actually explain this to users though. We no longer have 
hover on anything in the omnibox afaik (or won't soon), so you basically would see 
skull and crossbones vs gray https and have no idea what was going on if we divided 
UI for cases 1 and 2. If people didn't think it would be confusing I could get behind 
gray for mixed static content + red death for mixed active content (including CSS), 
but I would worry about it being confusing.

In the meantime, I think red skull and crossbones is probably a bit overkill for 
mixed content images, so until we actually can filter out the cases 1 and 2, I would 
say gray https is probably a safer way to go for now.
We're going to show mixed content errors in detail in the security info dialog if you 
click the icon.

My only worry about having the strongest indicators for e.g. mixed CSS is whether we'll 
train users to ignore it, if a lot of sites out there suffer from these problems.
The great thing about red skull + crossbones is that it highlights the site is broken 
and creates pressure to fix it.

Mixed CSS is currently treated as a content "display" not a content "ran" so it would 
not be subject to the strongest indicator.

(Aside: mixed CSS isn't as serious in Chrome as other browsers as we don't accept 
Javascript within CSS. There are some innovative attacks using CSS3 selectors to 
egress page content though).
"The great thing about red skull + crossbones is that it highlights the site is broken 
and creates pressure to fix it."

Be careful with assertions like this.  Users have no control over the site, and they do 
have choices in browsers, so unless all browsers do it, most users will believe the 
browser is broken, not the site.  (This is an extremely common argument over the entire 
life of the internet.  For a ludicrously extreme variant on this, see also "we should 
make HTML validation errors fatal to force sites to fix their code".)
Labels: UI-Needed Mstone-6
Labels: ReleaseBlock-Stable
Status: Assigned
Summary: Make mixed-content indicators correct (was: NULL)
OK, I claim this bug is now about:

* Mixed content warning -> "ghost padlock" + grey https
* Mixed content error -> skull + red https

Showing the precise errors in the info dialog is  issue 41549 .
 Issue 44114  has been merged into this issue.
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=47347 

------------------------------------------------------------------------
r47347 | pkasting@chromium.org | 2010-05-14 17:38:36 -0700 (Fri, 14 May 2010) | 15 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/app/generated_resources.grd?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/external_tab_container.cc?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/page_info_model.cc?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_browser_tests.cc?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_host_state.cc?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_host_state.h?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_host_state_unittest.cc?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_manager.cc?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_manager.h?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy.cc?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy.h?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy_backend.cc?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy_backend.h?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/tab_contents/navigation_entry.h?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/tab_contents/navigation_entry_unittest.cc?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/tab_contents/tab_contents.cc?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/tab_contents/tab_contents.h?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/toolbar_model.cc?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/notification_type.h?r1=47347&r2=47346
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/test/automation/automation_messages.h?r1=47347&r2=47346
   A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/test/data/ssl/page_displays_mixed_content.html
   A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/test/data/ssl/page_runs_mixed_content.html
   D /trunk/src/chrome/test/data/ssl/page_with_http_script.html
   D /trunk/src/chrome/test/data/ssl/page_with_mixed_contents.html
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome_frame/chrome_active_document.cc?r1=47347&r2=47346

(Original patch reviewed at http://codereview.chromium.org/2067003 )

Track "display" and "run" separately for mixed content, and make the latter downgrade the SSL state to "authentication broken".

Make the "display" state only affect the current tab (not the entire host).

Fix an SSL browser test by supplying the appropriate SiteInstance*.

Move a test from "disabled" to "flaky" since it at least passes for me.

Make the SSLManager header and .cc files put functions in the same order, and make that order somewhat saner.

BUG= 15072 ,  18626 ,  40932 ,  42758 
TEST=Covered by browser tests
Review URL: http://codereview.chromium.org/2063008
------------------------------------------------------------------------

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=47357 

------------------------------------------------------------------------
r47357 | rvargas@google.com | 2010-05-14 19:59:07 -0700 (Fri, 14 May 2010) | 29 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/app/generated_resources.grd?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/external_tab_container.cc?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/page_info_model.cc?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_browser_tests.cc?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_host_state.cc?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_host_state.h?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_host_state_unittest.cc?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_manager.cc?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_manager.h?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy.cc?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy.h?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy_backend.cc?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy_backend.h?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/tab_contents/navigation_entry.h?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/tab_contents/navigation_entry_unittest.cc?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/tab_contents/tab_contents.cc?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/tab_contents/tab_contents.h?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/toolbar_model.cc?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/notification_type.h?r1=47357&r2=47356
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/test/automation/automation_messages.h?r1=47357&r2=47356
   D /trunk/src/chrome/test/data/ssl/page_displays_mixed_content.html
   D /trunk/src/chrome/test/data/ssl/page_runs_mixed_content.html
   A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/test/data/ssl/page_with_http_script.html
   A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/test/data/ssl/page_with_mixed_contents.html
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome_frame/chrome_active_document.cc?r1=47357&r2=47356

Revert 47347 - (Original patch reviewed at http://codereview.chromium.org/2067003 )

Track "display" and "run" separately for mixed content, and make the latter downgrade the SSL state to "authentication broken".

Make the "display" state only affect the current tab (not the entire host).

Fix an SSL browser test by supplying the appropriate SiteInstance*.

Move a test from "disabled" to "flaky" since it at least passes for me.

Make the SSLManager header and .cc files put functions in the same order, and make that order somewhat saner.


Falied TestGoodFrameNavigation on Mac 10.5 browser tests

among other things:
/b/slave/chromium-rel-mac-builder/build/src/chrome/browser/ssl/ssl_browser_tests.cc:43: Failure
Value of: entry->ssl().displayed_mixed_content()
  Actual: false
Expected: displayed_mixed_content
Which is: true


BUG= 15072 ,  18626 ,  40932 ,  42758 
TEST=Covered by browser tests
Review URL: http://codereview.chromium.org/2063008

TBR=pkasting@chromium.org
Review URL: http://codereview.chromium.org/2095006
------------------------------------------------------------------------

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=47428 

------------------------------------------------------------------------
r47428 | pkasting@chromium.org | 2010-05-17 10:38:47 -0700 (Mon, 17 May 2010) | 16 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/app/generated_resources.grd?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/external_tab_container.cc?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/page_info_model.cc?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_browser_tests.cc?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_host_state.cc?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_host_state.h?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_host_state_unittest.cc?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_manager.cc?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_manager.h?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy.cc?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy.h?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy_backend.cc?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy_backend.h?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/tab_contents/navigation_entry.h?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/tab_contents/navigation_entry_unittest.cc?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/tab_contents/tab_contents.cc?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/tab_contents/tab_contents.h?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/toolbar_model.cc?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/notification_type.h?r1=47428&r2=47427
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/test/automation/automation_messages.h?r1=47428&r2=47427
   A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/test/data/ssl/page_displays_mixed_content.html
   A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/test/data/ssl/page_runs_mixed_content.html
   D /trunk/src/chrome/test/data/ssl/page_with_http_script.html
   D /trunk/src/chrome/test/data/ssl/page_with_mixed_contents.html
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome_frame/chrome_active_document.cc?r1=47428&r2=47427

Reland r47347 [was reverted in r47357], this time without re-enabling a DISABLED test that times out on Mac. (Original patch reviewed at http://codereview.chromium.org/2067003 )

Track "display" and "run" separately for mixed content, and make the latter downgrade the SSL state to "authentication broken".

Make the "display" state only affect the current tab (not the entire host).

Fix an SSL browser test by supplying the appropriate SiteInstance*.

Move a test from "disabled" to "flaky" since it at least passes for me.

Make the SSLManager header and .cc files put functions in the same order, and make that order somewhat saner.

BUG= 15072 ,  18626 ,  40932 ,  42758 
TEST=Covered by browser tests
Review URL: http://codereview.chromium.org/2063008
Review URL: http://codereview.chromium.org/2126005
------------------------------------------------------------------------

Status: Fixed
Fixed in r47428.
Comment 18 by Deleted ...@, May 27 2010
so why do I get red skulls when I access Gmail!!
Labels: -UI-Needed bulkmove Review-UI
For https with mixed content we show the https in the omnibox in gray. 
Visually this makes me think I shouldn't care (gray is usually used to de-
emphasize something). When I showed it to Glen he wasn't thrilled about it 
either and though we should be using red in this situation.
Project Member Comment 20 by bugdroid1@chromium.org, Oct 13 2012
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member Comment 21 by bugdroid1@chromium.org, Mar 10 2013
Labels: -Area-UI -Feature-Omnibox -Mstone-6 M-6 Cr-UI Cr-UI-Browser-Omnibox
Project Member Comment 22 by bugdroid1@chromium.org, Mar 13 2013
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Sign in to add a comment