Add SecurityStateModelClient interface and implementation

chrome/browser/ssl/security_state_model.cc

Index: chrome/browser/ssl/security_state_model.cc
diff --git a/chrome/browser/ssl/security_state_model.cc b/chrome/browser/ssl/security_state_model.cc
index ace1743eac733172c50365b7f95d418bf44ced38..b66225dd6f109a7012f68a0cbd0b3a8a56262eb0 100644
--- a/chrome/browser/ssl/security_state_model.cc
+++ b/chrome/browser/ssl/security_state_model.cc
@@ -9,6 +9,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ssl/chrome_security_state_model_delegate.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
@@ -115,7 +116,8 @@ SecurityStateModel::SecurityLevel GetSecurityLevelForRequest(
     Profile* profile,
     scoped_refptr<net::X509Certificate> cert,
     SecurityStateModel::SHA1DeprecationStatus sha1_status,
-    SecurityStateModel::MixedContentStatus mixed_content_status) {
+    SecurityStateModel::MixedContentStatus mixed_content_status,
+    bool used_known_mitm_certificate) {
   switch (ssl.security_style) {
     case content::SECURITY_STYLE_UNKNOWN:
       return SecurityStateModel::NONE;
@@ -134,17 +136,8 @@ SecurityStateModel::SecurityLevel GetSecurityLevelForRequest(
       return SecurityStateModel::SECURITY_WARNING;
 
     case content::SECURITY_STYLE_AUTHENTICATED: {
-#if defined(OS_CHROMEOS)
-      // Report if there is a policy cert first, before reporting any other
-      // authenticated-but-with-errors cases. A policy cert is a strong
-      // indicator of a MITM being present (the enterprise), while the
-      // other authenticated-but-with-errors indicate something may
-      // be wrong, or may be wrong in the future, but is unclear now.
-      policy::PolicyCertService* service =
-          policy::PolicyCertServiceFactory::GetForProfile(profile);
-      if (service && service->UsedPolicyCertificates())
+      if (used_known_mitm_certificate)
         return SecurityStateModel::SECURITY_POLICY_WARNING;
-#endif
 
       if (sha1_status == SecurityStateModel::DEPRECATED_SHA1_MAJOR)
         return SecurityStateModel::SECURITY_ERROR;
@@ -222,8 +215,9 @@ const SecurityStateModel::SecurityInfo& SecurityStateModel::GetSecurityInfo()
     // A cert must be present in the CertStore in order for the site to
     // be considered EV_SECURE, and the cert might have been removed
     // since the security level was last computed.
+    scoped_refptr<net::X509Certificate> cert;
     if (security_info_.security_level == EV_SECURE &&
-        !GetCertForSSLStatus(visible_ssl_status_)) {

[blundell, 2015/11/23 16:06:17]

Should this method be going away in this CL?

[estark, 2015/11/23 16:23:39]

Not quite yet, but it will be going away in one of the follow-up CLs. Here are
the gory details: GetCertForSSLStatus() is called from the static
SecurityInfoForRequest() on line 242, so it's still in use. However, because of
a recent, unrelated refactoring, the static SecurityInfoForRequest() method is
actually only needed for SecurityStateModel unit tests right now. Once I fill
out the SecurityStateModelDelegate interface, I'll be able to create a
TestSecurityStateModelDelegate in the unit tests and refactor the tests to use
that instead of relying on SecurityInfoForRequest() -- then I can get rid of
SecurityInfoForRequest() and GetCertForSSLStatus(). Does that make sense?

(In other words, right now the SSM unit tests construct content::SSLStatuses and
pass them in to SecurityInfoForRequest(), and soon I'll be able to instead
construct TestSSMDelegates and inject those into SSMs in the unit tests, as I
did in the original gigantic CL here:
https://codereview.chromium.org/1440303002/diff/280001/components/security_st...)

+        !delegate_->RetrieveCert(&cert)) {
       security_info_.security_level = SECURE;
     }
     return security_info_;
@@ -232,17 +226,19 @@ const SecurityStateModel::SecurityInfo& SecurityStateModel::GetSecurityInfo()
   SecurityInfoForRequest(
       entry->GetURL(), entry->GetSSL(),
       Profile::FromBrowserContext(web_contents_->GetBrowserContext()),
-      &security_info_);
+      delegate_->UsedKnownMITMCertificate(), &security_info_);
   visible_url_ = entry->GetURL();
   visible_ssl_status_ = entry->GetSSL();
   return security_info_;
 }
 
 // static
-void SecurityStateModel::SecurityInfoForRequest(const GURL& url,
-                                                const content::SSLStatus& ssl,
-                                                Profile* profile,
-                                                SecurityInfo* security_info) {
+void SecurityStateModel::SecurityInfoForRequest(
+    const GURL& url,
+    const content::SSLStatus& ssl,
+    Profile* profile,
+    bool used_known_mitm_certificate,
+    SecurityInfo* security_info) {
   scoped_refptr<net::X509Certificate> cert = GetCertForSSLStatus(ssl);
   security_info->cert_id = ssl.cert_id;
   security_info->sha1_deprecation_status = GetSHA1DeprecationStatus(cert, ssl);
@@ -264,8 +260,9 @@ void SecurityStateModel::SecurityInfoForRequest(const GURL& url,
 
   security_info->security_level = GetSecurityLevelForRequest(
       url, ssl, profile, cert, security_info->sha1_deprecation_status,
-      security_info->mixed_content_status);
+      security_info->mixed_content_status, used_known_mitm_certificate);
 }
 
 SecurityStateModel::SecurityStateModel(content::WebContents* web_contents)
-    : web_contents_(web_contents) {}
+    : web_contents_(web_contents),
+      delegate_(new ChromeSecurityStateModelDelegate(web_contents)) {}