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);
+  UpdateEntry(entry, SSLStatus::NORMAL_CONTENT);
   // 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, SSLStatus::NORMAL_CONTENT);
   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, SSLStatus::NORMAL_CONTENT);
   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,
+    SSLStatus::ContentStatusFlags 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;

[estark, 2016/10/12 07:01:19]

You might notice here that we used to clear flags in some cases in UpdateEntry()
and we no longer ever clear flags.

AFAICT this logic existed to deal with the odd navigation flow that I described
in the CL description. The flow before went like this:
1. Navigation commits, UpdateEntry() is called and uses the WebContentsImpl
flags from the previous navigation.
2. Post-commit tasks run, WebContentsImpl clears its flags, UpdateEntry() is
called again and now has to clear the flags that it set in step #1 because they
are no longer set on the WebContentsImpl.

Now, when a navigation commits, UpdateEntry() is called and does *not* read
WebContentsImpl flags from the previous navigation, so there is no need for a
step #2 to clear them.

(Other than that odd navigation flow, these flags never need to get cleared once
they are set on an SSLStatus.)

-  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(
+    SSLStatus::ContentStatusFlags 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) {
+  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(),
+                      SSLStatus::NORMAL_CONTENT);
+  }
+}
+
 }  // namespace content