Introduce PasswordManager.CertificateErrorsWhileSeeingForms metric

This CL introduces a UMA metric to measure the proportion of selected
SSL errors on pages with password forms.

Bug: 431618
Change-Id: I8344dab5744c5e5cd01820673aee17013ffa135c
Reviewed-on: https://chromium-review.googlesource.com/870322
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530459}

components/password_manager/core/browser/password_manager.cc

@@ -582,6 +582,30 @@ void PasswordManager::CreatePendingLoginManagers(
   // Record whether or not this top-level URL has at least one password field.
   client_->AnnotateNavigationEntry(!forms.empty());
 
+  // Only report SSL error status for cases where there are potentially forms to
+  // fill or save from.
+  if (!forms.empty()) {
+    metrics_util::CertificateError cert_error =
+        metrics_util::CertificateError::NONE;
+    const net::CertStatus cert_status = client_->GetMainFrameCertStatus();
+    // The order of the if statements matters -- if the status involves multiple
+    // errors, Chrome should report the one highest up in the list below.
+    if (cert_status & net::CERT_STATUS_AUTHORITY_INVALID)
+      cert_error = metrics_util::CertificateError::AUTHORITY_INVALID;
+    else if (cert_status & net::CERT_STATUS_COMMON_NAME_INVALID)
+      cert_error = metrics_util::CertificateError::COMMON_NAME_INVALID;
+    else if (cert_status & net::CERT_STATUS_WEAK_SIGNATURE_ALGORITHM)
+      cert_error = metrics_util::CertificateError::WEAK_SIGNATURE_ALGORITHM;
+    else if (cert_status & net::CERT_STATUS_DATE_INVALID)
+      cert_error = metrics_util::CertificateError::DATE_INVALID;
+    else if (net::IsCertStatusError(cert_status))
+      cert_error = metrics_util::CertificateError::OTHER;
+
+    UMA_HISTOGRAM_ENUMERATION(
+        "PasswordManager.CertificateErrorsWhileSeeingForms", cert_error,
+        metrics_util::CertificateError::COUNT);
+  }
+
   if (!client_->IsFillingEnabledForCurrentPage())
     return;
 

components/password_manager/core/browser/password_manager_metrics_util.h

@@ -263,6 +263,17 @@ enum ShowAllSavedPasswordsContext {
   SHOW_ALL_SAVED_PASSWORDS_CONTEXT_COUNT
 };
 
+// Metrics: "PasswordManager.CertificateErrorsWhileSeeingForms"
+enum class CertificateError {
+  NONE = 0,
+  OTHER = 1,
+  AUTHORITY_INVALID = 2,
+  DATE_INVALID = 3,
+  COMMON_NAME_INVALID = 4,
+  WEAK_SIGNATURE_ALGORITHM = 5,
+  COUNT
+};
+
 // A version of the UMA_HISTOGRAM_BOOLEAN macro that allows the |name|
 // to vary over the program's runtime.
 void LogUMAHistogramBoolean(const std::string& name, bool sample);

components/password_manager/core/browser/password_manager_unittest.cc

@@ -29,6 +29,7 @@
 #include "components/password_manager/core/browser/stub_password_manager_driver.h"
 #include "components/password_manager/core/common/password_manager_features.h"
 #include "components/password_manager/core/common/password_manager_pref_names.h"
+#include "net/cert/cert_status_flags.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -62,6 +63,7 @@ class MockPasswordManagerClient : public StubPasswordManagerClient {
   }
 
   MOCK_CONST_METHOD0(IsSavingAndFillingEnabledForCurrentPage, bool());
+  MOCK_CONST_METHOD0(GetMainFrameCertStatus, net::CertStatus());
   MOCK_CONST_METHOD0(GetPasswordStore, PasswordStore*());
   // The code inside EXPECT_CALL for PromptUserToSaveOrUpdatePasswordPtr and
   // ShowManualFallbackForSavingPtr owns the PasswordFormManager* argument.
@@ -154,6 +156,7 @@ class PasswordManagerTest : public testing::Test {
         .WillRepeatedly(Return(manager_.get()));
     EXPECT_CALL(driver_, GetPasswordAutofillManager())
         .WillRepeatedly(Return(password_autofill_manager_.get()));
+    EXPECT_CALL(client_, GetMainFrameCertStatus()).WillRepeatedly(Return(0));
 
     ON_CALL(client_, GetMainFrameURL()).WillByDefault(ReturnRef(test_url_));
   }
@@ -2207,4 +2210,60 @@ TEST_F(PasswordManagerTest, SaveSyncPasswordHashOnChangePasswordPage) {
   manager()->OnPasswordFormsRendered(&driver_, observed, true);
 }
 
+// If there are no forms to parse, certificate errors should not be reported.
+TEST_F(PasswordManagerTest, CertErrorReported_NoForms) {
+  const std::vector<PasswordForm> observed;
+  EXPECT_CALL(client_, GetMainFrameCertStatus())
+      .WillRepeatedly(Return(net::CERT_STATUS_AUTHORITY_INVALID));
+  EXPECT_CALL(*store_, GetLogins(_, _))
+      .WillRepeatedly(WithArg<1>(InvokeEmptyConsumerWithForms()));
+
+  base::HistogramTester histogram_tester;
+  manager()->OnPasswordFormsParsed(&driver_, observed);
+  histogram_tester.ExpectTotalCount(
+      "PasswordManager.CertificateErrorsWhileSeeingForms", 0);
+}
+
+TEST_F(PasswordManagerTest, CertErrorReported) {
+  constexpr struct {
+    net::CertStatus cert_status;
+    metrics_util::CertificateError expected_error;
+  } kCases[] = {
+      {0, metrics_util::CertificateError::NONE},
+      {net::CERT_STATUS_SHA1_SIGNATURE_PRESENT,  // not an error
+       metrics_util::CertificateError::NONE},
+      {net::CERT_STATUS_COMMON_NAME_INVALID,
+       metrics_util::CertificateError::COMMON_NAME_INVALID},
+      {net::CERT_STATUS_WEAK_SIGNATURE_ALGORITHM,
+       metrics_util::CertificateError::WEAK_SIGNATURE_ALGORITHM},
+      {net::CERT_STATUS_DATE_INVALID,
+       metrics_util::CertificateError::DATE_INVALID},
+      {net::CERT_STATUS_AUTHORITY_INVALID,
+       metrics_util::CertificateError::AUTHORITY_INVALID},
+      {net::CERT_STATUS_WEAK_KEY, metrics_util::CertificateError::OTHER},
+      {net::CERT_STATUS_DATE_INVALID | net::CERT_STATUS_WEAK_KEY,
+       metrics_util::CertificateError::DATE_INVALID},
+      {net::CERT_STATUS_DATE_INVALID | net::CERT_STATUS_AUTHORITY_INVALID,
+       metrics_util::CertificateError::AUTHORITY_INVALID},
+      {net::CERT_STATUS_DATE_INVALID | net::CERT_STATUS_AUTHORITY_INVALID |
+           net::CERT_STATUS_WEAK_KEY,
+       metrics_util::CertificateError::AUTHORITY_INVALID},
+  };
+
+  const std::vector<PasswordForm> observed = {PasswordForm()};
+
+  for (const auto& test_case : kCases) {
+    SCOPED_TRACE(testing::Message("index of test_case = ")
+                 << (&test_case - kCases));
+    EXPECT_CALL(client_, GetMainFrameCertStatus())
+        .WillRepeatedly(Return(test_case.cert_status));
+    base::HistogramTester histogram_tester;
+    EXPECT_CALL(*store_, GetLogins(_, _));
+    manager()->OnPasswordFormsParsed(&driver_, observed);
+    histogram_tester.ExpectUniqueSample(
+        "PasswordManager.CertificateErrorsWhileSeeingForms",
+        test_case.expected_error, 1);
+  }
+}
+
 }  // namespace password_manager

tools/metrics/histograms/enums.xml

@@ -33026,6 +33026,15 @@ Called by update_net_trust_anchors.py.-->
   <int value="2" label="No thanks"/>
 </enum>
 
+<enum name="PasswordCertificateError">
+  <int value="0" label="No error"/>
+  <int value="1" label="Other error"/>
+  <int value="2" label="Authority invalid (e.g., self-signed certificates)"/>
+  <int value="3" label="Date invalid (e.g., expired)"/>
+  <int value="4" label="Common name invalid"/>
+  <int value="5" label="Weak signature algorithm"/>
+</enum>
+
 <enum name="PasswordFormQueryVolume">
   <int value="0" label="New password query"/>
   <int value="1" label="Current query"/>

tools/metrics/histograms/histograms.xml

@@ -60229,6 +60229,20 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="PasswordManager.CertificateErrorsWhileSeeingForms"
+    enum="PasswordCertificateError">
+  <owner>battre@chromium.org</owner>
+  <owner>vabr@chromium.org</owner>
+  <summary>
+    When the password manager sees new forms on the page, it records in this
+    histogram whether there were any SSL certificate errors. The presence of SSL
+    errors likely means that the password manger will stop working, so the
+    reporting is done at the last point when password manager is still
+    guaranteed to be active. Some particular errors are distinguished, with the
+    rest being reported in a catch-all bucket.
+  </summary>
+</histogram>
+
 <histogram name="PasswordManager.EditsInSaveBubble"
     enum="PasswordManagerEditsInSaveBubbleEnum">
   <owner>battre@chromium.org</owner>