Expand SecurityStyleChanged interfaces to include explanations

chrome/browser/ssl/connection_security.cc

Index: chrome/browser/ssl/connection_security.cc
diff --git a/chrome/browser/ssl/connection_security.cc b/chrome/browser/ssl/connection_security.cc
index 12a27ad070d770c039da3df94d948902861be030..a1d732cd53a2a24a849cb9ac1b3350dbe3b9f9eb 100644
--- a/chrome/browser/ssl/connection_security.cc
+++ b/chrome/browser/ssl/connection_security.cc
@@ -71,6 +71,44 @@ connection_security::SecurityLevel GetSecurityLevelForNonSecureFieldTrial() {
   return level;
 }
 
+scoped_refptr<net::X509Certificate> GetCertForSSLStatus(
+    const content::SSLStatus& ssl) {
+  scoped_refptr<net::X509Certificate> cert;
+  if (content::CertStore::GetInstance()->RetrieveCert(ssl.cert_id, &cert))

[Peter Kasting, 2015/06/16 06:29:10]

Nit: Shorter:

  return content::CertStore::GetInstance()->RetrieveCert(ssl.cert_id, &cert) ?
      cert : nullptr;

[estark, 2015/06/16 15:32:34]

Done.

+    return cert;
+  return nullptr;
+}
+
+connection_security::SHA1DeprecationStatus GetSHA1DeprecationStatus(
+    scoped_refptr<net::X509Certificate> cert,
+    const content::SSLStatus& ssl) {
+  if (cert && (ssl.cert_status & net::CERT_STATUS_SHA1_SIGNATURE_PRESENT)) {
+    // The internal representation of the dates for UI treatment of SHA-1.
+    // See http://crbug.com/401365 for details.
+    static const int64_t kJanuary2017 = INT64_C(13127702400000000);
+    // kJanuary2016 needs to be kept in sync with
+    // ToolbarModelAndroid::IsDeprecatedSHA1Present().
+    static const int64_t kJanuary2016 = INT64_C(13096080000000000);

[Peter Kasting, 2015/06/16 06:29:11]

Nit: I'd move this second constant and its comment below the first conditional.

[estark, 2015/06/16 15:32:34]

Done.

+    if (cert->valid_expiry() >= base::Time::FromInternalValue(kJanuary2017)) {

[Peter Kasting, 2015/06/16 06:29:10]

Nit: No {} (2 places)

[estark, 2015/06/16 15:32:34]

Done.

+      return connection_security::DEPRECATED_SHA1_BROKEN;
+    }
+    if (cert->valid_expiry() >= base::Time::FromInternalValue(kJanuary2016)) {
+      return connection_security::DEPRECATED_SHA1_WARNING;
+    }
+  }
+
+  return connection_security::NO_DEPRECATED_SHA1;
+}
+
+connection_security::MixedContentStatus GetMixedContentStatus(
+    const content::SSLStatus& ssl) {
+  if (ssl.content_status & content::SSLStatus::DISPLAYED_INSECURE_CONTENT)
+    return connection_security::DISPLAYED_MIXED_CONTENT;
+  if (ssl.content_status & content::SSLStatus::RAN_INSECURE_CONTENT)

[Peter Kasting, 2015/06/16 06:29:11]

Since these are bitfield values and thus both could be set, shouldn't you check
RAN before DISPLAYED?

[estark, 2015/06/16 15:32:34]

Done.

+    return connection_security::RAN_MIXED_CONTENT;
+  return connection_security::NO_MIXED_CONTENT;
+}
+
 }  // namespace
 
 namespace connection_security {
@@ -108,26 +146,21 @@ SecurityLevel GetSecurityLevelForWebContents(
       if (service && service->UsedPolicyCertificates())
         return SECURITY_POLICY_WARNING;
 #endif
-      if (ssl.content_status & content::SSLStatus::DISPLAYED_INSECURE_CONTENT)
+
+      MixedContentStatus mixed_content_status = GetMixedContentStatus(ssl);
+      // Active mixed content is downgraded to the BROKEN style and
+      // handled above.
+      DCHECK(mixed_content_status != RAN_MIXED_CONTENT);

[Peter Kasting, 2015/06/16 06:29:11]

Nit: DCHECK_NE

[estark, 2015/06/16 15:32:34]

Done.

+      if (mixed_content_status == DISPLAYED_MIXED_CONTENT)
         return SECURITY_WARNING;
-      scoped_refptr<net::X509Certificate> cert;
-      if (content::CertStore::GetInstance()->RetrieveCert(ssl.cert_id, &cert) &&
-          (ssl.cert_status & net::CERT_STATUS_SHA1_SIGNATURE_PRESENT)) {
-        // The internal representation of the dates for UI treatment of SHA-1.
-        // See http://crbug.com/401365 for details.
-        static const int64_t kJanuary2017 = INT64_C(13127702400000000);
-        // kJanuary2016 needs to be kept in sync with
-        // ToolbarModelAndroid::IsDeprecatedSHA1Present().
-        static const int64_t kJanuary2016 = INT64_C(13096080000000000);
-        if (cert->valid_expiry() >=
-            base::Time::FromInternalValue(kJanuary2017)) {
-          return SECURITY_ERROR;
-        }
-        if (cert->valid_expiry() >=
-            base::Time::FromInternalValue(kJanuary2016)) {
-          return SECURITY_WARNING;
-        }
-      }
+
+      scoped_refptr<net::X509Certificate> cert = GetCertForSSLStatus(ssl);
+      SHA1DeprecationStatus sha1_status = GetSHA1DeprecationStatus(cert, ssl);
+      if (sha1_status == DEPRECATED_SHA1_BROKEN)
+        return SECURITY_ERROR;
+      else if (sha1_status == DEPRECATED_SHA1_WARNING)

[Peter Kasting, 2015/06/16 06:29:10]

Nit: No else after return

[estark, 2015/06/16 15:32:34]

Done.

+        return SECURITY_WARNING;
+
       if (net::IsCertStatusError(ssl.cert_status)) {
         DCHECK(net::IsCertStatusMinorError(ssl.cert_status));
         return SECURITY_WARNING;
@@ -148,25 +181,45 @@ SecurityLevel GetSecurityLevelForWebContents(
   }
 }
 
-content::SecurityStyle GetSecurityStyleForWebContents(
-    const content::WebContents* web_contents) {
-  SecurityLevel security_level = GetSecurityLevelForWebContents(web_contents);
+void GetSecurityInfoForWebContents(const content::WebContents* web_contents,
+                                   SecurityInfo* security_info) {
+  if (!web_contents) {

[Peter Kasting, 2015/06/16 06:29:11]

Nit: Shorter:

  content::NavigationEntry* entry =
      web_contents ? web_contents->GetController().GetVisibleEntry() : nullptr;
  if (!entry) {
    ...

[estark, 2015/06/16 15:32:34]

Done.

+    security_info->security_style = content::SECURITY_STYLE_UNKNOWN;
+    return;
+  }
+
+  content::NavigationEntry* entry =
+      web_contents->GetController().GetVisibleEntry();
+  if (!entry) {
+    security_info->security_style = content::SECURITY_STYLE_UNKNOWN;
+    return;
+  }
 
+  const content::SSLStatus& ssl = entry->GetSSL();

[Peter Kasting, 2015/06/16 06:29:10]

Nit: Declare this just above the first use below rather than here.

[estark, 2015/06/16 15:32:34]

Done.

+
+  SecurityLevel security_level = GetSecurityLevelForWebContents(web_contents);
   switch (security_level) {
     case NONE:
-      return content::SECURITY_STYLE_UNAUTHENTICATED;
+      security_info->security_style = content::SECURITY_STYLE_UNAUTHENTICATED;
+      break;
     case EV_SECURE:
     case SECURE:
-      return content::SECURITY_STYLE_AUTHENTICATED;
+      security_info->security_style = content::SECURITY_STYLE_AUTHENTICATED;
+      break;
     case SECURITY_WARNING:
     case SECURITY_POLICY_WARNING:
-      return content::SECURITY_STYLE_WARNING;
+      security_info->security_style = content::SECURITY_STYLE_WARNING;
+      break;
     case SECURITY_ERROR:
-      return content::SECURITY_STYLE_AUTHENTICATION_BROKEN;
+      security_info->security_style =
+          content::SECURITY_STYLE_AUTHENTICATION_BROKEN;
+      break;
   }
 
-  NOTREACHED();
-  return content::SECURITY_STYLE_UNKNOWN;
+  scoped_refptr<net::X509Certificate> cert = GetCertForSSLStatus(ssl);
+  security_info->sha1_deprecation_status = GetSHA1DeprecationStatus(cert, ssl);
+  security_info->mixed_content_status = GetMixedContentStatus(ssl);
+  security_info->cert_status = ssl.cert_status;
 }
 
 }  // namespace connection_security