Manage insecure content flags in SSLManager, not WebContentsImpl

content/browser/ssl/ssl_manager.cc

 // Copyright (c) 2012 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 "content/browser/ssl/ssl_manager.h"
 
 #include <set>
 
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/supports_user_data.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 #include "content/browser/loader/resource_request_info_impl.h"
 #include "content/browser/ssl/ssl_error_handler.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/certificate_request_result_type.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/ssl_host_state_delegate.h"
-#include "content/public/browser/ssl_status.h"
 #include "net/url_request/url_request.h"
 
 namespace content {
 
 namespace {
 
 const char kSSLManagerKeyName[] = "content_ssl_manager";
 
 // Events for UMA. Do not reorder or change!
 enum SSLGoodCertSeenEvent {
@@ skipping 108 matching lines @@
     int render_process_id,
     int render_frame_id,
     const net::SSLInfo& ssl_info,
     bool fatal) {
   OnSSLCertificateError(delegate, RESOURCE_TYPE_SUB_RESOURCE, url,
                         base::Bind(&WebContentsImpl::FromRenderFrameHostID,
                                    render_process_id, render_frame_id),
                         ssl_info, fatal);
 }
 
-// static
-void SSLManager::NotifySSLInternalStateChanged(BrowserContext* context) {
-  SSLManagerSet* managers = static_cast<SSLManagerSet*>(
-      context->GetUserData(kSSLManagerKeyName));
-
-  for (std::set<SSLManager*>::iterator i = managers->get().begin();
-       i != managers->get().end(); ++i) {
-    (*i)->UpdateEntry((*i)->controller()->GetLastCommittedEntry());
-  }
-}
-
 SSLManager::SSLManager(NavigationControllerImpl* controller)
     : controller_(controller),
       ssl_host_state_delegate_(
           controller->GetBrowserContext()->GetSSLHostStateDelegate()) {
   DCHECK(controller_);
 
   SSLManagerSet* managers = static_cast<SSLManagerSet*>(
       controller_->GetBrowserContext()->GetUserData(kSSLManagerKeyName));
   if (!managers) {
     managers = new SSLManagerSet;
     controller_->GetBrowserContext()->SetUserData(kSSLManagerKeyName, managers);
   }
   managers->get().insert(this);
 }
 
 SSLManager::~SSLManager() {
   SSLManagerSet* managers = static_cast<SSLManagerSet*>(
       controller_->GetBrowserContext()->GetUserData(kSSLManagerKeyName));
   managers->get().erase(this);
 }
 
 void SSLManager::DidCommitProvisionalLoad(const LoadCommittedDetails& details) {
   NavigationEntryImpl* entry = controller_->GetLastCommittedEntry();
-  UpdateEntry(entry);
+  int content_status_flags = 0;
+  if (!details.is_main_frame) {
+    // If it wasn't a main-frame navigation, then carry over content
+    // status flags. (For example, the mixed content flag shouldn't
+    // clear because of a frame navigation.)
+    NavigationEntryImpl* previous_entry =
+        controller_->GetEntryAtIndex(details.previous_entry_index);
+    if (previous_entry) {
+      content_status_flags = previous_entry->GetSSL().content_status;
+    }
+  }
+  UpdateEntry(entry, content_status_flags);
   // Always notify the WebContents that the SSL state changed when a
   // load is committed, in case the active navigation entry has changed.
   NotifyDidChangeVisibleSSLState();
 }
 
-void SSLManager::DidRunInsecureContent(const GURL& security_origin) {
+void SSLManager::DidDisplayMixedContent() {
+  UpdateLastCommittedEntry(SSLStatus::DISPLAYED_INSECURE_CONTENT);
+}
+
+void SSLManager::DidDisplayContentWithCertErrors() {
   NavigationEntryImpl* entry = controller_->GetLastCommittedEntry();
   if (!entry)
     return;
+  // Only record information about subresources with cert errors if the
+  // main page is HTTPS with a certificate.
+  if (entry->GetURL().SchemeIsCryptographic() && entry->GetSSL().certificate) {
+    UpdateLastCommittedEntry(SSLStatus::DISPLAYED_CONTENT_WITH_CERT_ERRORS);
+  }
+}
+
+void SSLManager::DidShowPasswordInputOnHttp() {
+  UpdateLastCommittedEntry(SSLStatus::DISPLAYED_PASSWORD_FIELD_ON_HTTP);
+}
+
+void SSLManager::DidShowCreditCardInputOnHttp() {
+  UpdateLastCommittedEntry(SSLStatus::DISPLAYED_CREDIT_CARD_FIELD_ON_HTTP);
+}
+
+void SSLManager::DidRunMixedContent(const GURL& security_origin) {
+  NavigationEntryImpl* entry = controller_->GetLastCommittedEntry();
+  if (!entry)
+    return;
 
   SiteInstance* site_instance = entry->site_instance();
   if (!site_instance)
     return;
 
   if (ssl_host_state_delegate_) {
     ssl_host_state_delegate_->HostRanInsecureContent(
         security_origin.host(), site_instance->GetProcess()->GetID(),
         SSLHostStateDelegate::MIXED_CONTENT);
   }
-  UpdateEntry(entry);
+  UpdateEntry(entry, 0);
   NotifySSLInternalStateChanged(controller_->GetBrowserContext());
 }
 
 void SSLManager::DidRunContentWithCertErrors(const GURL& security_origin) {
   NavigationEntryImpl* entry = controller_->GetLastCommittedEntry();
   if (!entry)
     return;
 
   SiteInstance* site_instance = entry->site_instance();
   if (!site_instance)
     return;
 
   if (ssl_host_state_delegate_) {
     ssl_host_state_delegate_->HostRanInsecureContent(
         security_origin.host(), site_instance->GetProcess()->GetID(),
         SSLHostStateDelegate::CERT_ERRORS_CONTENT);
   }
-  UpdateEntry(entry);
+  UpdateEntry(entry, 0);
   NotifySSLInternalStateChanged(controller_->GetBrowserContext());
 }
 
 void SSLManager::OnCertError(std::unique_ptr<SSLErrorHandler> handler) {
   bool expired_previous_decision = false;
   // First we check if we know the policy for this error.
   DCHECK(handler->ssl_info().is_valid());
   SSLHostStateDelegate::CertJudgment judgment =
       ssl_host_state_delegate_
           ? ssl_host_state_delegate_->QueryPolicy(
@@ skipping 91 matching lines @@
   const net::SSLInfo& ssl_info = handler->ssl_info();
   const GURL& request_url = handler->request_url();
   ResourceType resource_type = handler->resource_type();
   GetContentClient()->browser()->AllowCertificateError(
       web_contents, cert_error, ssl_info, request_url, resource_type,
       overridable, strict_enforcement, expired_previous_decision,
       base::Bind(&OnAllowCertificate, base::Owned(handler.release()),
                  ssl_host_state_delegate_));
 }
 
-void SSLManager::UpdateEntry(NavigationEntryImpl* entry) {
+void SSLManager::UpdateEntry(NavigationEntryImpl* entry,
+                             int additional_content_status_flags) {
   // We don't always have a navigation entry to update, for example in the
   // case of the Web Inspector.
   if (!entry)
     return;
 
   SSLStatus original_ssl_status = entry->GetSSL();  // Copy!
   entry->GetSSL().initialized = true;
-
-  WebContentsImpl* web_contents_impl =
-      static_cast<WebContentsImpl*>(controller_->delegate()->GetWebContents());
-
-  // For sensitive inputs (password, credit card) on HTTP, do not clear
-  // the |content_status| flag when the WebContents no longer has the
-  // flag set. This is different from how DISPLAYED_INSECURE_CONTENT and
-  // DISPLAYED_CONTENT_WITH_CERT_ERRORS are handled below. For sensitive
-  // inputs on HTTP, once the NavigationEntry has been marked as having
-  // displayed a sensitive input, it stays that way, even if the
-  // sensitive input is subsequently removed from the page.
-  if (web_contents_impl->DisplayedPasswordFieldOnHttp()) {
-    entry->GetSSL().content_status |=
-        SSLStatus::DISPLAYED_PASSWORD_FIELD_ON_HTTP;
-  }
-
-  if (web_contents_impl->DisplayedCreditCardFieldOnHttp()) {
-    entry->GetSSL().content_status |=
-        SSLStatus::DISPLAYED_CREDIT_CARD_FIELD_ON_HTTP;
-  }
-
-  // Do not record information about insecure subresources if the main
-  // page is HTTP or HTTPS without a certificate.
-  if (!entry->GetURL().SchemeIsCryptographic() || !entry->GetSSL().certificate)
-    return;
-
-  // Update the entry's flags for insecure content.
-  if (!web_contents_impl->DisplayedInsecureContent())
-    entry->GetSSL().content_status &= ~SSLStatus::DISPLAYED_INSECURE_CONTENT;
-  if (web_contents_impl->DisplayedInsecureContent())
-    entry->GetSSL().content_status |= SSLStatus::DISPLAYED_INSECURE_CONTENT;
-  if (!web_contents_impl->DisplayedContentWithCertErrors()) {
-    entry->GetSSL().content_status &=
-        ~SSLStatus::DISPLAYED_CONTENT_WITH_CERT_ERRORS;
-  }
-  if (web_contents_impl->DisplayedContentWithCertErrors()) {
-    entry->GetSSL().content_status |=
-        SSLStatus::DISPLAYED_CONTENT_WITH_CERT_ERRORS;
-  }
+  entry->GetSSL().content_status |= additional_content_status_flags;
 
   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 insecure content.  See bug http://crbug.com/12423.
   if (site_instance && ssl_host_state_delegate_ &&
       ssl_host_state_delegate_->DidHostRunInsecureContent(
           entry->GetURL().host(), site_instance->GetProcess()->GetID(),
           SSLHostStateDelegate::MIXED_CONTENT)) {
     entry->GetSSL().content_status |= SSLStatus::RAN_INSECURE_CONTENT;
   }
 
-  if (site_instance && ssl_host_state_delegate_ &&
+  // Only record information about subresources with cert errors if the
+  // main page is HTTPS with a certificate.
+  if (entry->GetURL().SchemeIsCryptographic() && entry->GetSSL().certificate &&
+      site_instance && ssl_host_state_delegate_ &&
       ssl_host_state_delegate_->DidHostRunInsecureContent(
           entry->GetURL().host(), site_instance->GetProcess()->GetID(),
           SSLHostStateDelegate::CERT_ERRORS_CONTENT)) {
     entry->GetSSL().content_status |= SSLStatus::RAN_CONTENT_WITH_CERT_ERRORS;
   }
 
   if (!entry->GetSSL().Equals(original_ssl_status))
     NotifyDidChangeVisibleSSLState();
 }
 
+void SSLManager::UpdateLastCommittedEntry(int additional_content_status_flags) {
+  NavigationEntryImpl* entry = controller_->GetLastCommittedEntry();
+  if (!entry)
+    return;
+  UpdateEntry(entry, additional_content_status_flags);
+}
+
 void SSLManager::NotifyDidChangeVisibleSSLState() {
   WebContentsImpl* contents =
       static_cast<WebContentsImpl*>(controller_->delegate()->GetWebContents());
   contents->DidChangeVisibleSSLState();
 }
 
+// static
+void SSLManager::NotifySSLInternalStateChanged(BrowserContext* context) {

[estark, 2016/10/12 19:21:26]

A note about why this is private now: we were previously calling this for any
state changed, but I don't think that's necessary; we only need to call this for
state changes that can affect other tabs, namely |ssl_host_state_delegate|
changes. Since all relevant state changes go directly to SSLManager now,
SSLManager can internally decide when NotifySSLInternalStateChanged() needs to
be called.

+  SSLManagerSet* managers =
+      static_cast<SSLManagerSet*>(context->GetUserData(kSSLManagerKeyName));
+
+  for (std::set<SSLManager*>::iterator i = managers->get().begin();
+       i != managers->get().end(); ++i) {
+    (*i)->UpdateEntry((*i)->controller()->GetLastCommittedEntry(), 0);
+  }
+}
+
 }  // namespace content