Attach a SecurityStyle to each request in ResourceLoader

content/browser/ssl/ssl_policy.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_policy.h"
 
 #include "base/base_switches.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/memory/singleton.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/browser/ssl/ssl_cert_error_handler.h"
 #include "content/browser/ssl/ssl_request_info.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/content_browser_client.h"
+#include "content/public/browser/web_contents.h"
 #include "content/public/common/resource_type.h"
 #include "content/public/common/ssl_status.h"
 #include "content/public/common/url_constants.h"
 #include "net/ssl/ssl_info.h"
-
+#include "url/gurl.h"
 
 namespace content {
 
 namespace {
 
 // Events for UMA. Do not reorder or change!
 enum SSLGoodCertSeenEvent {
   NO_PREVIOUS_EXCEPTION = 0,
   HAD_PREVIOUS_EXCEPTION = 1,
   SSL_GOOD_CERT_SEEN_EVENT_MAX = 2
@@ skipping 94 matching lines @@
       // certificates.
       backend_->RevokeUserAllowExceptions(info->url().host());
       event = HAD_PREVIOUS_EXCEPTION;
     }
     UMA_HISTOGRAM_ENUMERATION("interstitial.ssl.good_cert_seen", event,
                               SSL_GOOD_CERT_SEEN_EVENT_MAX);
   }
 }
 
 void SSLPolicy::UpdateEntry(NavigationEntryImpl* entry,
-                            WebContentsImpl* web_contents) {
+                            WebContents* web_contents) {
   DCHECK(entry);
 
   InitializeEntryIfNeeded(entry);
 
-  if (!entry->GetURL().SchemeIsCryptographic())
+  if (entry->GetSSL().security_style == SECURITY_STYLE_UNAUTHENTICATED)
     return;
 
   if (!web_contents->DisplayedInsecureContent())
     entry->GetSSL().content_status &= ~SSLStatus::DISPLAYED_INSECURE_CONTENT;
 
-  // 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->GetSSL().cert_id) {
-    entry->GetSSL().security_style = SECURITY_STYLE_UNAUTHENTICATED;
-    return;
-  }
-
   if (web_contents->DisplayedInsecureContent())
     entry->GetSSL().content_status |= SSLStatus::DISPLAYED_INSECURE_CONTENT;
 
-  if (net::IsCertStatusError(entry->GetSSL().cert_status)) {
-    // Minor errors don't lower the security style to
-    // SECURITY_STYLE_AUTHENTICATION_BROKEN.
-    if (!net::IsCertStatusMinorError(entry->GetSSL().cert_status)) {
-      entry->GetSSL().security_style =
-          SECURITY_STYLE_AUTHENTICATION_BROKEN;
-    }
+  if (entry->GetSSL().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 insecure content.  See bug http://crbug.com/12423.
   if (site_instance &&
       backend_->DidHostRunInsecureContent(
           entry->GetURL().host(), site_instance->GetProcess()->GetID())) {
     entry->GetSSL().security_style =
         SECURITY_STYLE_AUTHENTICATION_BROKEN;
     entry->GetSSL().content_status |= SSLStatus::RAN_INSECURE_CONTENT;
     return;
   }
 }
 
+// Static
+SecurityStyle SSLPolicy::GetSecurityStyleForResource(const GURL& url,
+                                                     const SSLStatus& ssl) {
+  // 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 (!url.SchemeIsCryptographic() || !ssl.cert_id)
+    return SECURITY_STYLE_UNAUTHENTICATED;
+
+  if (net::IsCertStatusError(ssl.cert_status)) {
+    // Minor errors don't lower the security style to
+    // SECURITY_STYLE_AUTHENTICATION_BROKEN.
+    if (!net::IsCertStatusMinorError(ssl.cert_status))

[davidben, 2015/07/22 20:56:57]

Nit: You can fold this to

// Minor blah blah
if (net::IsCertStatusError(...) &&
    !net::IsCertStatusMinorError(...)) {
  return ...
}

[estark, 2015/07/22 22:56:55]

Done.

+      return SECURITY_STYLE_AUTHENTICATION_BROKEN;
+  }
+
+  return SECURITY_STYLE_AUTHENTICATED;
+}
+
 void SSLPolicy::OnAllowCertificate(scoped_refptr<SSLCertErrorHandler> handler,
                                    bool allow) {
   DCHECK(handler->ssl_info().is_valid());
   if (allow) {
     // Default behavior for accepting a certificate.
     // Note that we should not call SetMaxSecurityStyle here, because the active
     // NavigationEntry has just been deleted (in HideInterstitialPage) and the
     // new NavigationEntry will not be set until DidNavigate.  This is ok,
     // because the new NavigationEntry will have its max security style set
     // within DidNavigate.
@@ skipping 47 matching lines @@
       break;
     default:
       NOTREACHED();
   }
 }
 
 void SSLPolicy::InitializeEntryIfNeeded(NavigationEntryImpl* entry) {
   if (entry->GetSSL().security_style != SECURITY_STYLE_UNKNOWN)
     return;
 
-  entry->GetSSL().security_style = entry->GetURL().SchemeIsCryptographic()
-                                       ? SECURITY_STYLE_AUTHENTICATED
-                                       : SECURITY_STYLE_UNAUTHENTICATED;
+  entry->GetSSL().security_style =
+      GetSecurityStyleForResource(entry->GetURL(), entry->GetSSL());

[davidben, 2015/07/22 20:56:57]

Do you know if this code can ever run, now that security_style flows through the
renderer instead? Maybe for the initial state of things?

[Checked codesearch and it doesn't appear possible to update cert_id or
cert_status and not security_style, which would be one way this might change
behavior. Before, UpdateEntry would pick up the implications for security_style
while this new code won't. But it doesn't seem that ever happens, so it should
be fine. Not updating them in tandem seems scary.]

[estark, 2015/07/22 22:56:55]

I think the only way it can run right now is 1. a misbehaving renderer (which,
as you pointed out elsewhere, we should probably disallow), or 2. if it is
somehow possible for the first committed navigation to not be a |is_main_frame|
navigation. The latter sounds like a weird thing but I can't find any evidence
that it's impossible.

 }
 
 void SSLPolicy::OriginRanInsecureContent(const std::string& origin, int pid) {
   GURL parsed_origin(origin);
   if (parsed_origin.SchemeIsCryptographic())
     backend_->HostRanInsecureContent(parsed_origin.host(), pid);
 }
 
 }  // namespace content