Issue 8706 - chromium - Mixed content warning can be removed - An open-source project to help move the web forward.

Issue 8706:	Mixed content warning can be removed
	2 people starred this issue and may be notified of changes.	Back to list

Status:	Verified
Owner:	abarth@chromium.org
Closed:	Mar 2009
Cc:	abarth@chromium.org, jcam...@chromium.org, wtc@chromium.org, jap...@chromium.org
Security OS-All Pri-2 Area-BrowserUI Type-Bug-Security Securit…Severity-Low Security_Impact-Stable

Restricted

Only users with Commit permission may comment.

Reported by adhi...@google.com, Mar 12, 2009

When vising an HTTPS URL, if the page includes any content over HTTP, then 
a warning icon is displayed instead of the lock icon.  However the 
insecure HTTP content can trick Chrome into removing the mixed content 
warning.

To reproduce:

1) Ensure Chrome is set to allow mixed content, which is the default. The 
setting is options->under the hood->When there is mixed content->allow all 
content to load.

2) Go to the demo at https://adhintzaetest.appspot.com/mixed.html

Expected behavior:  The mixed content warning icon is displayed because a 
script loaded over HTTP has access to the page.

Actual behavior: The SSL lock icon is displayed.

What appears to happen:
This HTTPS page includes a script via HTTP.  When loaded you may briefly 
see the mixed content warning icon.  The HTTP script overwrites 
document.body.innerHTML and then sets location.hash to a new value.  This 
causes Chrome to see re-check if there's mixed content.  Because there's 
no mention of the HTTP script in the new document.body.innerHTML, Chrome 
displays the trusted lock icon.

Mar 12, 2009

#1 lcam...@gmail.com

Nice find. Note that the whole assumption that mixed content is a property that goes
away with page transitions is a very dodgy one. See this test for Chrome, which quite
frankly, I'm not sure how to address:

https://[internal WWW]/~lcamtuf/mixed_c1.html

The same attack actually works in Firefox and MSIE 7, though Chrome pop-up behavior
definitely makes it worse.

Mar 12, 2009

#2 wtc@chromium.org

Adam, Jay, could you take a look at this bug?

Status: Untriaged
Cc: aba...@chromium.org jcam...@chromium.org

Mar 12, 2009

#3 abarth@chromium.org

Yes.  We've known about this bug (and its variants) for a while.  The fix is the 
cache the mixed content state per render process / security origin pair.  Currently, 
we only cache the info per page, which is too narrow.

I'm making as P2 because every browser has this bug.  We need to fix it, but there 
isn't any reason to hold up the beta release.

Owner: aba...@chromium.org
Labels: -Pri-0 -Area-Misc Pri-2 Area-BrowserUI

Mar 12, 2009

#4 lcam...@gmail.com

Caching it across site visits sounds like a good way to annoy / confuse web
developers, though maybe for the better.

Also note that this would probably require expanding the notion of mixed content; for
example, I can now display mixed content https:// data within an IFRAME in a http://
document without triggering the warning, but https:// content is effectively
compromised past this point.

Mar 12, 2009

#5 abarth@chromium.org

Yes.  I think we catch this stuff so we can apply our "don't load mixed content" 
pref.  It's just a matter of recording it in the cache.

Mar 14, 2009

#6 abarth@chromium.org

Patch in progress.

Status: Started

Mar 15, 2009

#7 abarth@chromium.org

This is requiring a substantial re-organization of how we compute our mixed content 
state.  We might want to hold off on other changes to the browser/ssl/* files while I 
sort this out.

Cc: w...@chromium.org

Mar 15, 2009

#8 abarth@chromium.org

Here is a work in progress patch:

http://codereview.chromium.org/46094

It doesn't compile or anything, but you can see the general direction its heading.

Mar 16, 2009

#9 abarth@chromium.org

Updated work-in-progress.  This is mostly in shape now.  Next step: run tests.

Mar 16, 2009

#10 abarth@chromium.org

I've split up that monster patch into several smaller steps.  Hopefully, I'll be 
landing some of these tonight:

Step 1: http://codereview.chromium.org/48034
Step 2: http://codereview.chromium.org/42255
Step 3: http://codereview.chromium.org/48038
Step 4: http://codereview.chromium.org/42263

None of these patches should have any functional changes.  Functional changes will 
come in another round.  :)

Mar 16, 2009

#11 bugdroid1@gmail.com

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

------------------------------------------------------------------------
r11838 | abarth@chromium.org | 2009-03-16 21:13:22 -0700 (Mon, 16 Mar 2009) | 9 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/webkit/glue/webframe.h?r1=11838&r2=11837
   M http://src.chromium.org/viewvc/chrome/trunk/src/webkit/glue/webframe_impl.cc?r1=11838&r2=11837
   M http://src.chromium.org/viewvc/chrome/trunk/src/webkit/glue/webframe_impl.h?r1=11838&r2=11837

SSLPolicy Fix: Step 1.

Expose some more accessors on WebFrame.

R=wtc
BUG=8706


Review URL: http://codereview.chromium.org/48034
------------------------------------------------------------------------

Mar 16, 2009

#12 bugdroid1@gmail.com

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

------------------------------------------------------------------------
r11839 | abarth@chromium.org | 2009-03-16 21:22:18 -0700 (Mon, 16 Mar 2009) | 2 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/base/x509_certificate.cc?r1=11839&r2=11838
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/base/x509_certificate.h?r1=11839&r2=11838

SSL Fix: Step 2.Add HasAllowedCert and HasDeniedCert to X509Certificate::Policy.R=wtcBUG=8706
Review URL: http://codereview.chromium.org/42255
------------------------------------------------------------------------

Mar 16, 2009

#13 bugdroid1@gmail.com

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

------------------------------------------------------------------------
r11850 | abarth@chromium.org | 2009-03-16 23:11:08 -0700 (Mon, 16 Mar 2009) | 9 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/load_from_memory_cache_details.h?r1=11850&r2=11849
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/render_view_host.cc?r1=11850&r2=11849
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/render_view_host.h?r1=11850&r2=11849
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/render_view_host_delegate.h?r1=11850&r2=11849
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/tab_contents/web_contents.cc?r1=11850&r2=11849
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/tab_contents/web_contents.h?r1=11850&r2=11849
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/render_messages_internal.h?r1=11850&r2=11849
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/render_view.cc?r1=11850&r2=11849

SSL Fix: Step 4.

Plumb the security origin information for memory cache loads too.

R=wtc
BUG=8706


Review URL: http://codereview.chromium.org/42263
------------------------------------------------------------------------

Mar 17, 2009

#14 bugdroid1@gmail.com

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

------------------------------------------------------------------------
r11851 | abarth@chromium.org | 2009-03-17 02:18:06 -0700 (Tue, 17 Mar 2009) | 8 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/resource_dispatcher_host.cc?r1=11851&r2=11850
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/resource_dispatcher_host.h?r1=11851&r2=11850
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/resource_dispatcher_host_unittest.cc?r1=11851&r2=11850
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/resource_request_details.h?r1=11851&r2=11850
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_manager.cc?r1=11851&r2=11850
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_manager.h?r1=11851&r2=11850
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy.cc?r1=11851&r2=11850
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy.h?r1=11851&r2=11850
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/render_messages.h?r1=11851&r2=11850
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/resource_dispatcher.cc?r1=11851&r2=11850
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/resource_dispatcher.h?r1=11851&r2=11850
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/resource_dispatcher_unittest.cc?r1=11851&r2=11850
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/plugin/chrome_plugin_host.cc?r1=11851&r2=11850
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/media/data_source_impl.cc?r1=11851&r2=11850
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/renderer_glue.cc?r1=11851&r2=11850
   M http://src.chromium.org/viewvc/chrome/trunk/src/webkit/glue/resource_handle_impl.cc?r1=11851&r2=11850
   M http://src.chromium.org/viewvc/chrome/trunk/src/webkit/glue/resource_loader_bridge.h?r1=11851&r2=11850
   M http://src.chromium.org/viewvc/chrome/trunk/src/webkit/glue/unittest_test_server.h?r1=11851&r2=11850
   M http://src.chromium.org/viewvc/chrome/trunk/src/webkit/tools/test_shell/simple_resource_loader_bridge.cc?r1=11851&r2=11850

SSLPolicy Fix: Step 3.

Plumbing the security origin of the frame making the request to SSL land.

R=wtc
BUG=8706

Review URL: http://codereview.chromium.org/48038
------------------------------------------------------------------------

Mar 17, 2009

#15 bugdroid1@gmail.com

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

------------------------------------------------------------------------
r11853 | abarth@chromium.org | 2009-03-17 03:08:24 -0700 (Tue, 17 Mar 2009) | 9 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/browser.cc?r1=11853&r2=11852
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_manager.cc?r1=11853&r2=11852
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy.cc?r1=11853&r2=11852
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/notification_type.h?r1=11853&r2=11852

SSLPolicy Fix: Step 5.

Add a new notification type to notify other SSLManagers when a security origin becomes contaminated with mixed content.

TBR=wtc
BUG=8706


Review URL: http://codereview.chromium.org/48059
------------------------------------------------------------------------

Mar 17, 2009

#16 abarth@chromium.org

Also r11854 for missing ssl_blocking_page.cc in last patch.

Mar 17, 2009

#17 abarth@chromium.org

Also r11880 where Mark fixed an uninitialized variable.

Mar 17, 2009

#18 bugdroid1@gmail.com

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

------------------------------------------------------------------------
r11891 | abarth@chromium.org | 2009-03-17 11:56:23 -0700 (Tue, 17 Mar 2009) | 10 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_host_state.cc?r1=11891&r2=11890
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_host_state.h?r1=11891&r2=11890
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_manager.cc?r1=11891&r2=11890

SSLPolicy Fix: Step 6.

Merge in changes to SSLHostState.  We now can store whether a specific origin is "broken," which is the key new bit of state that we need to share between tabs.

Currently, there is a naming inconsistency between the SSLManager names and the SSLHostState names.  I'll clear this up when I merge in the new SSLManager.

R=jcampan
BUG=8706

Review URL: http://codereview.chromium.org/42274
------------------------------------------------------------------------

Mar 17, 2009

#19 bugdroid1@gmail.com

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

------------------------------------------------------------------------
r11892 | abarth@chromium.org | 2009-03-17 11:56:42 -0700 (Tue, 17 Mar 2009) | 8 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy.cc?r1=11892&r2=11891
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy.h?r1=11892&r2=11891

SSLPolicy Fix: Step 7.

Simplify SSLPolicy to prepare for changing its algorithm.  This change should not change the SSLPolicy behavior at all.

R=jcampan
BUG=8706

Review URL: http://codereview.chromium.org/48060
------------------------------------------------------------------------

Mar 17, 2009

#20 bugdroid1@gmail.com

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

------------------------------------------------------------------------
r11937 | abarth@chromium.org | 2009-03-17 18:07:07 -0700 (Tue, 17 Mar 2009) | 8 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_manager.cc?r1=11937&r2=11936
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_manager.h?r1=11937&r2=11936
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy.cc?r1=11937&r2=11936
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy.h?r1=11937&r2=11936

SSLPolicy Fix: Step 8.

Cleanup the SSLPolicy API.  This should be the last reorganization patch.  The next step should be the substantive changes.

R=jcampan
BUG=8706

Review URL: http://codereview.chromium.org/48091
------------------------------------------------------------------------

Mar 17, 2009

#21 abarth@chromium.org

Also r11938 to remove a DCHECK that snuck in.

Mar 18, 2009

#22 abarth@chromium.org

Final step out for review here:

http://codereview.chromium.org/42314

Mar 19, 2009

#23 bugdroid1@gmail.com

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

------------------------------------------------------------------------
r12178 | abarth@chromium.org | 2009-03-19 18:37:47 -0700 (Thu, 19 Mar 2009) | 9 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/resource_request_details.h?r1=12178&r2=12177
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_host_state.cc?r1=12178&r2=12177
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_host_state.h?r1=12178&r2=12177
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_manager.cc?r1=12178&r2=12177
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_manager.h?r1=12178&r2=12177
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy.cc?r1=12178&r2=12177
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_policy.h?r1=12178&r2=12177
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_uitest.cc?r1=12178&r2=12177
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/tab_contents/navigation_controller.cc?r1=12178&r2=12177
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/tab_contents/navigation_entry.h?r1=12178&r2=12177
   A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/test/data/ssl/blank_page.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/randomize_hash.js

SSLPolicy fix: Step 9 of 9 (hopefully!).

Change our algorithm for computing the state of our SSL security indicators.  Previously, we were computing this state for a single navigation entry.  Although this matches other browsers, it fails to take the same-origin policy into account.  For example, if one tab is contaminated with insecure content, that insecure content can spread to all the tabs in the same security origin.

R=jcampan,wtc
BUG=8706
TEST=SSLUITest.TestMixedContentsRandomizeHash,SSLUITest.TestMixedContentsTwoTabs

Review URL: http://codereview.chromium.org/42314
------------------------------------------------------------------------

Mar 19, 2009

#24 abarth@chromium.org

Fixed.  I'm not sure if Drew knew what he was getting into when he reported this bug.  
:)  There are still some minor issues, but I'll deal with them in follow-up bugs.

Status: Fixed

Mar 21, 2009

#25 adhi...@google.com

:)  Thanks for fixing the issue!

Mar 23, 2009

#26 mberkow...@chromium.org

Verified in 2.0.171.0 (Developer Build 12281).

Status: Verified

Apr 21, 2010

#27 abarth@chromium.org

(No comment was entered for this change.)

Cc: jap...@chromium.org

Jan 4, 2011

#28 jschuh@chromium.org

(No comment was entered for this change.)

Labels: SecSeverity-Low

Mar 21, 2011

#29 jschuh@chromium.org

(No comment was entered for this change.)

Labels: Type-Security

Oct 5, 2011

#30 jschuh@chromium.org

Batch update: assuming these security changes impacted stable based on some fuzzy filtering.

Labels: SecImpacts-Stable

Apr 18, 2012

#31 jschuh@chromium.org

Lifting view restrictions.

Labels: -private

Oct 13, 2012

#32 bugdroid1@chromium.org

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.

Labels: Restrict-AddIssueComment-Commit

Mar 9, 2013

#33 bugdroid1@chromium.org

(No comment was entered for this change.)

Labels: -SecSeverity-Low -Type-Security -SecImpacts-Stable Security-Severity-Low Security-Impact-Stable Type-Bug-Security

Mar 21, 2013

#34 bugdroid1@chromium.org

(No comment was entered for this change.)

Labels: -Security-Severity-Low Security_Severity-Low

Mar 21, 2013

#35 bugdroid1@chromium.org

(No comment was entered for this change.)

Labels: -Security-Impact-Stable Security_Impact-Stable

► Sign in to add a comment