Track "display" and "run" separately for mixed content, and make the latter d...

chrome/browser/ssl/ssl_policy.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ssl/ssl_policy.h"
 
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
 #include "base/base_switches.h"
 #include "base/command_line.h"
@@ skipping 65 matching lines @@
     case net::ERR_CERT_INVALID:
       OnCertErrorInternal(handler, SSLBlockingPage::ERROR_FATAL);
       break;
     default:
       NOTREACHED();
       handler->CancelRequest();
       break;
   }
 }
 
-void SSLPolicy::DidDisplayInsecureContent(NavigationEntry* entry) {
-  if (!entry)
-    return;
-
-  // TODO(abarth): We don't actually need to break the whole origin here,
-  //               but we can handle that in a later patch.
-  DidRunInsecureContent(entry, entry->url().spec());
-}
-
 void SSLPolicy::DidRunInsecureContent(NavigationEntry* entry,
                                       const std::string& security_origin) {
   if (!entry)
     return;
 
   SiteInstance* site_instance = entry->site_instance();
   if (!site_instance)
       return;
 
-  backend_->MarkHostAsBroken(GURL(security_origin).host(),
-                             site_instance->GetProcess()->id());
+  backend_->HostRanInsecureContent(GURL(security_origin).host(),
+                                   site_instance->GetProcess()->id());
 }
 
 void SSLPolicy::OnRequestStarted(SSLRequestInfo* info) {
-  if (net::IsCertStatusError(info->ssl_cert_status()))
-    UpdateStateForUnsafeContent(info);
+  // TODO(abarth): This mechanism is wrong.  What we should be doing is sending
+  // this information back through WebKit and out some FrameLoaderClient
+  // methods.
+  //
+  // The behavior for HTTPS resources with cert errors should be as follows:
+  //   1) If we don't know anything about this host (the one hosting the
+  //      resource), the resource load just fails.
+  //   2) If the user has previously approved the same certificate error for
+  //      this host in a full-page interstitial, then we'll proceed with the
+  //      load.
+  //   3) If we proceed with the load, we should treat the resources as if they
+  //      were loaded over HTTP, w.r.t. the display vs. run distinction above.
+  //
+  // However, right now we don't have the proper context to understand where
+  // these resources will be used.  Consequently, we're conservative and treat
+  // them all like DidRunInsecureContent().
+
+  if (net::IsCertStatusError(info->ssl_cert_status())) {
+    backend_->HostRanInsecureContent(info->url().host(), info->child_id());
+
+    // TODO(abarth): We should eventually remove the main_frame_origin and
+    // frame_origin properties.  First, not every resource load is associated
+    // with a frame, so they don't always make sense.  Second, the
+    // main_frame_origin is computed from the first_party_for_cookies, which has
+    // been hacked to death to support third-party cookie blocking.
+
+    if (info->resource_type() != ResourceType::MAIN_FRAME &&
+        info->resource_type() != ResourceType::SUB_FRAME) {
+      // The frame's origin now contains mixed content and therefore is broken.
+      OriginRanInsecureContent(info->frame_origin(), info->child_id());
+    }
+
+    if (info->resource_type() != ResourceType::MAIN_FRAME) {
+      // The main frame now contains a frame with mixed content.  Therefore, we
+      // mark the main frame's origin as broken too.
+      OriginRanInsecureContent(info->main_frame_origin(), info->child_id());
+    }
+  }
 }
 
-void SSLPolicy::UpdateEntry(NavigationEntry* entry) {
+void SSLPolicy::UpdateEntry(NavigationEntry* entry, TabContents* tab_contents) {
   DCHECK(entry);
 
   InitializeEntryIfNeeded(entry);
 
   if (!entry->url().SchemeIsSecure())
     return;
 
   // An HTTPS response may not have a certificate for some reason.  When that
   // happens, use the unauthenticated (HTTP) rather than the authentication
   // broken security style so that we can detect this error condition.
   if (!entry->ssl().cert_id()) {
     entry->ssl().set_security_style(SECURITY_STYLE_UNAUTHENTICATED);
     return;
   }
 
   // If CERT_STATUS_UNABLE_TO_CHECK_REVOCATION is the only certificate error,
   // don't lower the security style to SECURITY_STYLE_AUTHENTICATION_BROKEN.
   int cert_errors = entry->ssl().cert_status() & net::CERT_STATUS_ALL_ERRORS;
   if (cert_errors) {
     if (cert_errors != net::CERT_STATUS_UNABLE_TO_CHECK_REVOCATION)
       entry->ssl().set_security_style(SECURITY_STYLE_AUTHENTICATION_BROKEN);
     return;
   }
 
   SiteInstance* site_instance = entry->site_instance();
   // Note that |site_instance| can be NULL here because NavigationEntries don't
   // necessarily have site instances.  Without a process, the entry can't
   // possibly have mixed content.  See bug http://crbug.com/12423.
   if (site_instance &&
-      backend_->DidMarkHostAsBroken(entry->url().host(),
-                                    site_instance->GetProcess()->id()))
-    entry->ssl().set_has_mixed_content();
+      backend_->DidHostRunInsecureContent(entry->url().host(),
+                                          site_instance->GetProcess()->id())) {
+    entry->ssl().set_security_style(SECURITY_STYLE_AUTHENTICATION_BROKEN);
+    entry->ssl().set_ran_mixed_content();

[abarth-chromium, 2010/05/14 22:29:39]

It's a bit strange that things get renamed from "insecure" to "mixed" at this
point.  I'm not sure that's worth worrying too much about.

[Peter Kasting, 2010/05/14 22:33:11]

I'd be happy to try for consistent naming if you have a preference.  Should I
remove "mixed content" everywhere and make it all "insecure content"?

[abarth-chromium, 2010/05/14 22:35:03]

Sure.  I think that's the path of least resistance for a consistent name.

+    return;
+  }
+
+  if (tab_contents->displayed_insecure_content())
+    entry->ssl().set_displayed_mixed_content();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // SSLBlockingPage::Delegate methods
 
 SSLErrorInfo SSLPolicy::GetSSLErrorInfo(SSLCertErrorHandler* handler) {
   return SSLErrorInfo::CreateError(
       SSLErrorInfo::NetErrorToErrorType(handler->cert_error()),
       handler->ssl_info().cert, handler->request_url());
 }
@@ skipping 44 matching lines @@
 }
 
 void SSLPolicy::InitializeEntryIfNeeded(NavigationEntry* entry) {
   if (entry->ssl().security_style() != SECURITY_STYLE_UNKNOWN)
     return;
 
   entry->ssl().set_security_style(entry->url().SchemeIsSecure() ?
       SECURITY_STYLE_AUTHENTICATED : SECURITY_STYLE_UNAUTHENTICATED);
 }
 
-void SSLPolicy::MarkOriginAsBroken(const std::string& origin, int pid) {
+void SSLPolicy::OriginRanInsecureContent(const std::string& origin, int pid) {
   GURL parsed_origin(origin);
-  if (!parsed_origin.SchemeIsSecure())
-    return;
-
-  backend_->MarkHostAsBroken(parsed_origin.host(), pid);
+  if (parsed_origin.SchemeIsSecure())
+    backend_->HostRanInsecureContent(parsed_origin.host(), pid);
 }
-
-void SSLPolicy::UpdateStateForMixedContent(SSLRequestInfo* info) {
-  // TODO(abarth): This function isn't right because we need to remove
-  //               info->main_frame_origin().
-
-  if (info->resource_type() != ResourceType::MAIN_FRAME &&
-      info->resource_type() != ResourceType::SUB_FRAME) {
-    // The frame's origin now contains mixed content and therefore is broken.
-    MarkOriginAsBroken(info->frame_origin(), info->child_id());
-  }
-
-  if (info->resource_type() != ResourceType::MAIN_FRAME) {
-    // The main frame now contains a frame with mixed content.  Therefore, we
-    // mark the main frame's origin as broken too.
-    MarkOriginAsBroken(info->main_frame_origin(), info->child_id());
-  }
-}
-
-void SSLPolicy::UpdateStateForUnsafeContent(SSLRequestInfo* info) {
-  // This request as a broken cert, which means its host is broken.
-  backend_->MarkHostAsBroken(info->url().host(), info->child_id());
-  UpdateStateForMixedContent(info);
-}