[M90] PSM: Record different results of PSM and Hash dance

PSM vs Hash dance comparison was showing small percentage that both have
different results. That case should almost not happening. Therefore, we
are verifying if some data is missing for PSM database by getting more
results as PSM false and Hash dance is true.

This CL is adding a new comparison UMA that is recording the actual
protocols values when they are different.

See: go/psm-hashdance-diff-values

BUG=chromium:1184411

(cherry picked from commit 15f904b)

(cherry picked from commit 7e967be)

Change-Id: Ifa42b735d6dd7de4fb798905482a2049838e389c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2732036
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Amr Aboelkher <amraboelkher@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/master@{#859976}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2739630
Reviewed-by: Amr Aboelkher <amraboelkher@google.com>
Cr-Original-Commit-Position: refs/branch-heads/4423@{#3}
Cr-Original-Branched-From: 2344957-refs/heads/master@{#855676}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2742491
Cr-Commit-Position: refs/branch-heads/4430@{#222}
Cr-Branched-From: e5ce7dc-refs/heads/master@{#857950}

chrome/browser/chromeos/policy/auto_enrollment_client_impl.cc

@@ -1384,6 +1384,18 @@ void AutoEnrollmentClientImpl::RecordPsmHashDanceComparison() {
     comparison = (hash_dance_decision == psm_decision.value())
                      ? PsmHashDanceComparison::kEqualResults
                      : PsmHashDanceComparison::kDifferentResults;
+
+    if (hash_dance_decision != psm_decision.value()) {
+      // Reports the different values of the protocols, after both have finished
+      // executing successfully.
+      auto different_protocols_results_comparison =
+          (psm_decision.value()
+               ? PsmHashDanceDifferentResultsComparison::kPsmTrueHashDanceFalse
+               : PsmHashDanceDifferentResultsComparison::
+                     kHashDanceTruePsmFalse);
+      base::UmaHistogramEnumeration(kUMAPsmHashDanceDifferentResultsComparison,
+                                    different_protocols_results_comparison);
+    }
   } else if (hash_dance_error && !psm_error) {
     comparison = PsmHashDanceComparison::kPSMSuccessHashDanceError;
   } else if (!hash_dance_error && psm_error) {

chrome/browser/chromeos/policy/auto_enrollment_client_impl.h

@@ -68,6 +68,16 @@ enum class PsmHashDanceComparison {
   kMaxValue = kBothError,
 };
 
+// Indicates all possible different results of PSM and Hash dance protocols,
+// after both protocols have executed successfully. These values are persisted
+// to logs. Entries should not be renumbered and numeric values should never be
+// reused.
+enum class PsmHashDanceDifferentResultsComparison {
+  kHashDanceTruePsmFalse = 0,
+  kPsmTrueHashDanceFalse = 1,
+  kMaxValue = kPsmTrueHashDanceFalse,
+};
+
 // Interacts with the device management service and determines whether this
 // machine should automatically enter the Enterprise Enrollment screen during
 // OOBE.

chrome/browser/chromeos/policy/auto_enrollment_client_impl_unittest.cc

@@ -1569,6 +1569,17 @@ class PsmHelperTest : public AutoEnrollmentClientImplTest {
                                          /*expected_count=*/1);
   }
 
+  // Expects a sample for kUMAPsmHashDanceDifferentResultsComparison to be
+  // recorded once with value |different_results_comparison|.
+  void ExpectPsmHashDanceDifferentResultsComparisonRecorded(
+      PsmHashDanceDifferentResultsComparison different_results_comparison)
+      const {
+    histogram_tester_.ExpectUniqueSample(
+        kUMAPsmHashDanceDifferentResultsComparison,
+        different_results_comparison,
+        /*expected_count=*/1);
+  }
+
   void VerifyPsmLastRequestJobType() const {
     EXPECT_EQ(DeviceManagementService::JobConfiguration::
                   TYPE_PSM_HAS_DEVICE_STATE_REQUEST,
@@ -1942,6 +1953,12 @@ TEST_P(PsmHelperAndHashDanceTest, PsmSucceedAndHashDanceSucceed) {
           ? PsmHashDanceComparison::kEqualResults
           : PsmHashDanceComparison::kDifferentResults);
 
+  if (GetExpectedMembershipResult() != kExpectedHashDanceResult) {
+    // Verify recorded different results for PSM and Hash dance.
+    ExpectPsmHashDanceDifferentResultsComparisonRecorded(
+        PsmHashDanceDifferentResultsComparison::kHashDanceTruePsmFalse);
+  }
+
   // Verify device state result.
   EXPECT_EQ(auto_enrollment_job_type_,
             DeviceManagementService::JobConfiguration::TYPE_AUTO_ENROLLMENT);
@@ -1989,6 +2006,12 @@ TEST_P(PsmHelperAndHashDanceTest,
           ? PsmHashDanceComparison::kEqualResults
           : PsmHashDanceComparison::kDifferentResults);
 
+  if (GetExpectedMembershipResult() != kExpectedHashDanceResult) {
+    // Verify recorded different results for PSM and Hash dance.
+    ExpectPsmHashDanceDifferentResultsComparisonRecorded(
+        PsmHashDanceDifferentResultsComparison::kPsmTrueHashDanceFalse);
+  }
+
   // Verify that no enrollment has been done, and no state has been retrieved.
   EXPECT_EQ(auto_enrollment_job_type_,
             DeviceManagementService::JobConfiguration::TYPE_AUTO_ENROLLMENT);
@@ -2092,6 +2115,12 @@ TEST_P(PsmHelperAndHashDanceTest,
       (GetExpectedMembershipResult() == kExpectedHashDanceResult)
           ? PsmHashDanceComparison::kEqualResults
           : PsmHashDanceComparison::kDifferentResults);
+
+  if (GetExpectedMembershipResult() != kExpectedHashDanceResult) {
+    // Verify recorded different results for PSM and Hash dance.
+    ExpectPsmHashDanceDifferentResultsComparisonRecorded(
+        PsmHashDanceDifferentResultsComparison::kHashDanceTruePsmFalse);
+  }
 }
 
 TEST_P(PsmHelperAndHashDanceTest,

components/policy/core/common/cloud/enterprise_metrics.cc

@@ -185,6 +185,8 @@ const char kMetricCBCMUnsignedRemoteCommandExecutedTemplate[] =
 
 const char kUMAPsmHashDanceComparison[] =
     "Enterprise.AutoEnrollmentPrivateSetMembershipHashDanceComparison";
+const char kUMAPsmHashDanceDifferentResultsComparison[] =
+    "Enterprise.AutoEnrollmentPsmHashDanceDifferentResultsComparison";
 const char kUMAPsmSuccessTime[] =
     "Enterprise.AutoEnrollmentPrivateSetMembershipSuccessTime";
 const char kUMAPsmRequestStatus[] =

components/policy/core/common/cloud/enterprise_metrics.h

@@ -259,6 +259,7 @@ POLICY_EXPORT extern const char
 
 // Private set membership UMA histogram names.
 POLICY_EXPORT extern const char kUMAPsmHashDanceComparison[];
+POLICY_EXPORT extern const char kUMAPsmHashDanceDifferentResultsComparison[];
 POLICY_EXPORT extern const char kUMAPsmSuccessTime[];
 POLICY_EXPORT extern const char kUMAPsmRequestStatus[];
 

tools/metrics/histograms/enums.xml

@@ -63413,6 +63413,11 @@ https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_2.7.1.pdf
   <int value="4" label="PROXY_HAS_RULES"/>
 </enum>
 
+<enum name="PsmHashDanceDifferentResultsComparison">
+  <int value="0" label="Hash dance true and PSM false"/>
+  <int value="1" label="PSM true and Hash dance false"/>
+</enum>
+
 <enum name="PublicKeyPinFailedDomain">
   <int value="0" label="DOMAIN_NOT_PINNED"/>
   <int value="1" label="DOMAIN_GOOGLE_COM"/>

tools/metrics/histograms/histograms_xml/enterprise/histograms.xml

@@ -148,6 +148,18 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   <summary>Total duration time of the auto-enrollment protocol.</summary>
 </histogram>
 
+<histogram
+    name="Enterprise.AutoEnrollmentPsmHashDanceDifferentResultsComparison"
+    enum="PsmHashDanceDifferentResultsComparison" expires_after="2021-09-01">
+  <owner>amraboelkher@google.com</owner>
+  <owner>mpolzer@google.com</owner>
+  <summary>
+    Comparison of hash dance and private set membership results used to
+    determine the initial enrollment state of the device. Only recorded after
+    both protocols have been executed successfully with different results.
+  </summary>
+</histogram>
+
 <histogram base="true" name="Enterprise.AutoEnrollmentRequestNetworkErrorCode"
     enum="NetErrorCodes" expires_after="2021-08-15">
 <!-- Name completed by histogram_suffixes name="EnterpriseAutoEnrollmentType". -->