bug 1521983 - remove some unused certificate pinning telemetry probes r=jcj,ulfr
authorDana Keeler <dkeeler@mozilla.com>
Mon, 04 Mar 2019 20:30:47 +0000
changeset 520150 dd200b211b4adbbf149096dcf7341576384c5129
parent 520149 8cbf86043273811dd463d6707945ca3346ade726
child 520151 15d98a1290fa75aa119120ad3b591dff90956bc4
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjcj, ulfr
bugs1521983
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 1521983 - remove some unused certificate pinning telemetry probes r=jcj,ulfr Differential Revision: https://phabricator.services.mozilla.com/D19731
security/manager/ssl/PublicKeyPinningService.cpp
security/manager/ssl/StaticHPKPins.h
security/manager/ssl/tests/unit/test_pinning.js
security/manager/tools/PreloadedHPKPins.json
toolkit/components/telemetry/Histograms.json
toolkit/components/telemetry/histogram-whitelists.json
--- a/security/manager/ssl/PublicKeyPinningService.cpp
+++ b/security/manager/ssl/PublicKeyPinningService.cpp
@@ -152,24 +152,43 @@ class TransportSecurityPreloadBinarySear
 
 nsresult PublicKeyPinningService::ChainMatchesPinset(
     const RefPtr<nsNSSCertList>& certList,
     const nsTArray<nsCString>& aSHA256keys,
     /*out*/ bool& chainMatchesPinset) {
   return EvalChain(certList, nullptr, &aSHA256keys, chainMatchesPinset);
 }
 
+#ifdef DEBUG
+static Atomic<bool> sValidatedPinningPreloadList(false);
+
+static void ValidatePinningPreloadList() {
+  if (sValidatedPinningPreloadList) {
+    return;
+  }
+  for (const auto& entry : kPublicKeyPinningPreloadList) {
+    // If and only if a static entry is a Mozilla entry, it has a telemetry ID.
+    MOZ_ASSERT((entry.mIsMoz && entry.mId != kUnknownId) ||
+               (!entry.mIsMoz && entry.mId == kUnknownId));
+  }
+  sValidatedPinningPreloadList = true;
+}
+#endif  // DEBUG
+
 // Returns via one of the output parameters the most relevant pinning
 // information that is valid for the given host at the given time.
 // Dynamic pins are prioritized over static pins.
 static nsresult FindPinningInformation(
     const char* hostname, mozilla::pkix::Time time,
     const OriginAttributes& originAttributes,
     /*out*/ nsTArray<nsCString>& dynamicFingerprints,
     /*out*/ const TransportSecurityPreload*& staticFingerprints) {
+#ifdef DEBUG
+  ValidatePinningPreloadList();
+#endif
   if (!hostname || hostname[0] == 0) {
     return NS_ERROR_INVALID_ARG;
   }
   staticFingerprints = nullptr;
   dynamicFingerprints.Clear();
   nsCOMPtr<nsISiteSecurityService> sssService =
       do_GetService(NS_SSSERVICE_CONTRACTID);
   if (!sssService) {
@@ -268,61 +287,66 @@ static nsresult CheckPinsForHostname(
   if (staticFingerprints) {
     bool enforceTestModeResult;
     rv = EvalChain(certList, staticFingerprints->pinset, nullptr,
                    enforceTestModeResult);
     if (NS_FAILED(rv)) {
       return rv;
     }
     chainHasValidPins = enforceTestModeResult;
-    Telemetry::HistogramID histogram = staticFingerprints->mIsMoz
-                                           ? Telemetry::CERT_PINNING_MOZ_RESULTS
-                                           : Telemetry::CERT_PINNING_RESULTS;
-    if (staticFingerprints->mTestMode) {
-      histogram = staticFingerprints->mIsMoz
-                      ? Telemetry::CERT_PINNING_MOZ_TEST_RESULTS
-                      : Telemetry::CERT_PINNING_TEST_RESULTS;
-      if (!enforceTestMode) {
-        chainHasValidPins = true;
+    if (staticFingerprints->mTestMode && !enforceTestMode) {
+      chainHasValidPins = true;
+    }
+
+    if (pinningTelemetryInfo) {
+      // If and only if a static entry is a Mozilla entry, it has a telemetry
+      // ID.
+      if ((staticFingerprints->mIsMoz &&
+           staticFingerprints->mId == kUnknownId) ||
+          (!staticFingerprints->mIsMoz &&
+           staticFingerprints->mId != kUnknownId)) {
+        return NS_ERROR_FAILURE;
       }
-    }
-    // We can collect per-host pinning violations for this host because it is
-    // operationally critical to Firefox.
-    if (pinningTelemetryInfo) {
-      if (staticFingerprints->mId != kUnknownId) {
-        int32_t bucket =
-            staticFingerprints->mId * 2 + (enforceTestModeResult ? 1 : 0);
+
+      Telemetry::HistogramID histogram;
+      int32_t bucket;
+      // We can collect per-host pinning violations for this host because it is
+      // operationally critical to Firefox.
+      if (staticFingerprints->mIsMoz) {
         histogram = staticFingerprints->mTestMode
                         ? Telemetry::CERT_PINNING_MOZ_TEST_RESULTS_BY_HOST
                         : Telemetry::CERT_PINNING_MOZ_RESULTS_BY_HOST;
-        pinningTelemetryInfo->certPinningResultBucket = bucket;
+        bucket = staticFingerprints->mId * 2 + (enforceTestModeResult ? 1 : 0);
       } else {
-        pinningTelemetryInfo->certPinningResultBucket =
-            enforceTestModeResult ? 1 : 0;
+        histogram = staticFingerprints->mTestMode
+                        ? Telemetry::CERT_PINNING_TEST_RESULTS
+                        : Telemetry::CERT_PINNING_RESULTS;
+        bucket = enforceTestModeResult ? 1 : 0;
       }
       pinningTelemetryInfo->accumulateResult = true;
       pinningTelemetryInfo->certPinningResultHistogram = Some(histogram);
-    }
-
-    // We only collect per-CA pinning statistics upon failures.
-    nsCOMPtr<nsIX509Cert> rootCert;
-    rv = certList->GetRootCertificate(rootCert);
-    if (NS_FAILED(rv)) {
-      return rv;
-    }
+      pinningTelemetryInfo->certPinningResultBucket = bucket;
 
-    // Only log telemetry if the certificate list is non-empty.
-    if (rootCert && !enforceTestModeResult && pinningTelemetryInfo) {
-      UniqueCERTCertificate rootCertObj =
-          UniqueCERTCertificate(rootCert.get()->GetCert());
-      if (rootCertObj) {
-        int32_t binNumber = RootCABinNumber(&rootCertObj->derCert);
-        if (binNumber != ROOT_CERTIFICATE_UNKNOWN) {
-          pinningTelemetryInfo->accumulateForRoot = true;
-          pinningTelemetryInfo->rootBucket = binNumber;
+      // We only collect per-CA pinning statistics upon failures.
+      if (!enforceTestModeResult) {
+        nsCOMPtr<nsIX509Cert> rootCert;
+        rv = certList->GetRootCertificate(rootCert);
+        if (NS_FAILED(rv)) {
+          return rv;
+        }
+        if (rootCert) {
+          UniqueCERTCertificate rootCertObj(rootCert->GetCert());
+          if (!rootCertObj) {
+            return NS_ERROR_FAILURE;
+          }
+          int32_t binNumber = RootCABinNumber(&rootCertObj->derCert);
+          if (binNumber != ROOT_CERTIFICATE_UNKNOWN) {
+            pinningTelemetryInfo->accumulateForRoot = true;
+            pinningTelemetryInfo->rootBucket = binNumber;
+          }
         }
       }
     }
 
     MOZ_LOG(gPublicKeyPinningLog, LogLevel::Debug,
             ("pkpin: Pin check %s for %s host '%s' (mode=%s)\n",
              enforceTestModeResult ? "passed" : "failed",
              staticFingerprints->mIsMoz ? "mozilla" : "non-mozilla", hostname,
--- a/security/manager/ssl/StaticHPKPins.h
+++ b/security/manager/ssl/StaticHPKPins.h
@@ -704,18 +704,18 @@ static const TransportSecurityPreload kP
   { "br.search.yahoo.com", false, true, false, -1, &kPinset_yahoo },
   { "bugs.chromium.org", true, false, false, -1, &kPinset_google_root_pems },
   { "build.chromium.org", true, false, false, -1, &kPinset_google_root_pems },
   { "business.facebook.com", true, false, false, -1, &kPinset_facebook },
   { "business.twitter.com", true, false, false, -1, &kPinset_twitterCom },
   { "ca.search.yahoo.com", false, true, false, -1, &kPinset_yahoo },
   { "cd.search.yahoo.com", false, true, false, -1, &kPinset_yahoo },
   { "cdn.ampproject.org", true, false, false, -1, &kPinset_google_root_pems },
-  { "cdn.mozilla.net", true, false, true, -1, &kPinset_mozilla_services },
-  { "cdn.mozilla.org", true, false, true, -1, &kPinset_mozilla_services },
+  { "cdn.mozilla.net", true, false, true, 16, &kPinset_mozilla_services },
+  { "cdn.mozilla.org", true, false, true, 17, &kPinset_mozilla_services },
   { "cg.search.yahoo.com", false, true, false, -1, &kPinset_yahoo },
   { "ch.search.yahoo.com", false, true, false, -1, &kPinset_yahoo },
   { "chart.apis.google.com", true, false, false, -1, &kPinset_google_root_pems },
   { "check.torproject.org", true, false, false, -1, &kPinset_tor },
   { "checkout.google.com", true, false, false, -1, &kPinset_google_root_pems },
   { "chfr.search.yahoo.com", false, true, false, -1, &kPinset_yahoo },
   { "chit.search.yahoo.com", false, true, false, -1, &kPinset_yahoo },
   { "chrome-devtools-frontend.appspot.com", true, false, false, -1, &kPinset_google_root_pems },
@@ -759,17 +759,17 @@ static const TransportSecurityPreload kP
   { "dropboxstatic.com", false, true, false, -1, &kPinset_dropbox },
   { "dropboxusercontent.com", false, true, false, -1, &kPinset_dropbox },
   { "edit.yahoo.com", true, true, false, -1, &kPinset_yahoo },
   { "en-maktoob.search.yahoo.com", false, true, false, -1, &kPinset_yahoo },
   { "encrypted.google.com", true, false, false, -1, &kPinset_google_root_pems },
   { "es.search.yahoo.com", false, true, false, -1, &kPinset_yahoo },
   { "espanol.search.yahoo.com", false, true, false, -1, &kPinset_yahoo },
   { "example.test", true, true, false, -1, &kPinset_test },
-  { "exclude-subdomains.pinning.example.com", false, false, false, 0, &kPinset_mozilla_test },
+  { "exclude-subdomains.pinning.example.com", false, false, false, -1, &kPinset_mozilla_test },
   { "facebook.com", false, false, false, -1, &kPinset_facebook },
   { "fi.google.com", true, false, false, -1, &kPinset_google_root_pems },
   { "fi.search.yahoo.com", false, true, false, -1, &kPinset_yahoo },
   { "firebaseio.com", true, false, false, -1, &kPinset_google_root_pems },
   { "firefox.com", true, true, true, 15, &kPinset_mozilla_services },
   { "fj.search.yahoo.com", false, true, false, -1, &kPinset_yahoo },
   { "fr.search.yahoo.com", false, true, false, -1, &kPinset_yahoo },
   { "g.co", true, false, false, -1, &kPinset_google_root_pems },
--- a/security/manager/ssl/tests/unit/test_pinning.js
+++ b/security/manager/ssl/tests/unit/test_pinning.js
@@ -210,42 +210,23 @@ function check_pinning_telemetry() {
   let prod_histogram = Services.telemetry.getHistogramById("CERT_PINNING_RESULTS")
                          .snapshot();
   let test_histogram = Services.telemetry.getHistogramById("CERT_PINNING_TEST_RESULTS")
                          .snapshot();
   // Because all of our test domains are pinned to user-specified trust
   // anchors, effectively only strict mode and enforce test-mode get evaluated
   equal(prod_histogram.values[0], 4,
         "Actual and expected prod (non-Mozilla) failure count should match");
-  equal(prod_histogram.values[1], 4,
+  equal(prod_histogram.values[1], 6,
         "Actual and expected prod (non-Mozilla) success count should match");
   equal(test_histogram.values[0], 2,
         "Actual and expected test (non-Mozilla) failure count should match");
   equal(test_histogram.values[1] || 0, 0,
         "Actual and expected test (non-Mozilla) success count should match");
 
-  let moz_prod_histogram = Services.telemetry.getHistogramById("CERT_PINNING_MOZ_RESULTS")
-                             .snapshot();
-  let moz_test_histogram =
-    Services.telemetry.getHistogramById("CERT_PINNING_MOZ_TEST_RESULTS").snapshot();
-  equal(moz_prod_histogram.values[0] || 0, 0,
-        "Actual and expected prod (Mozilla) failure count should match");
-  equal(moz_prod_histogram.values[1] || 0, 0,
-        "Actual and expected prod (Mozilla) success count should match");
-  equal(moz_test_histogram.values[0] || 0, 0,
-        "Actual and expected test (Mozilla) failure count should match");
-  equal(moz_test_histogram.values[1] || 0, 0,
-        "Actual and expected test (Mozilla) success count should match");
-
-  let per_host_histogram =
-    Services.telemetry.getHistogramById("CERT_PINNING_MOZ_RESULTS_BY_HOST").snapshot();
-  equal(per_host_histogram.values[0] || 0, 0,
-        "Actual and expected per host (Mozilla) failure count should match");
-  equal(per_host_histogram.values[1], 2,
-        "Actual and expected per host (Mozilla) success count should match");
   run_next_test();
 }
 
 function run_test() {
   add_tls_server_setup("BadCertServer", "bad_certs");
 
   // Add a user-specified trust anchor.
   addCertFromFile(certdb, "bad_certs/other-test-ca.pem", "CTu,u,u");
--- a/security/manager/tools/PreloadedHPKPins.json
+++ b/security/manager/tools/PreloadedHPKPins.json
@@ -183,20 +183,21 @@
     // superseded by catchall for firefox.com, but leaving for tracking
     { "name": "accounts.firefox.com", "include_subdomains": true,
       "pins": "mozilla_services", "test_mode": false, "id": 4 },
     { "name": "api.accounts.firefox.com", "include_subdomains": true,
       "pins": "mozilla_services", "test_mode": false, "id": 5 },
     { "name": "sync.services.mozilla.com", "include_subdomains": true,
       "pins": "mozilla_services", "test_mode": false, "id": 13 },
     // Catch-all for all CDN resources, including product delivery
+    // Telemetry IDs added in bug 1521983.
     { "name": "cdn.mozilla.net", "include_subdomains": true,
-      "pins": "mozilla_services", "test_mode": false },
+      "pins": "mozilla_services", "test_mode": false, "id": 16 },
     { "name": "cdn.mozilla.org", "include_subdomains": true,
-      "pins": "mozilla_services", "test_mode": false },
+      "pins": "mozilla_services", "test_mode": false, "id": 17 },
     { "name": "download.mozilla.org", "include_subdomains": false,
       "pins": "mozilla_services", "test_mode": false, "id": 14 },
     // Catch-all for everything hosted under services.mozilla.com
     { "name": "services.mozilla.com", "include_subdomains": true,
       "pins": "mozilla_services", "test_mode": false, "id": 6 },
     // Catch-all for everything hosted under telemetry.mozilla.org
     // MUST remain in test mode in order to receive telemetry on broken pins
     { "name": "telemetry.mozilla.org", "include_subdomains": true,
@@ -213,17 +214,17 @@
     { "name": "crash-stats.mozilla.org", "include_subdomains": false,
       "pins": "mozilla_services", "test_mode": false, "id": 12 },
     { "name": "include-subdomains.pinning.example.com",
       "include_subdomains": true, "pins": "mozilla_test",
       "test_mode": false },
     // Example domain to collect per-host stats for telemetry tests.
     { "name": "exclude-subdomains.pinning.example.com",
       "include_subdomains": false, "pins": "mozilla_test",
-      "test_mode": false, "id": 0 },
+      "test_mode": false },
     { "name": "test-mode.pinning.example.com", "include_subdomains": true,
       "pins": "mozilla_test", "test_mode": true },
     // Expand twitter's pinset to include all of *.twitter.com and use
     // twitterCDN. More specific rules take precedence because we search for
     // exact domain name first.
     { "name": "twitter.com", "include_subdomains": true,
       "pins": "twitterCDN", "test_mode": false }
   ],
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -11192,30 +11192,16 @@
   },
   "CERT_PINNING_TEST_RESULTS": {
     "record_in_processes": ["main", "content"],
     "alert_emails": ["pinning@mozilla.org"],
     "expires_in_version": "never",
     "kind": "boolean",
     "description": "Certificate pinning test results (0 = failure, 1 = success)"
   },
-  "CERT_PINNING_MOZ_RESULTS": {
-    "record_in_processes": ["main", "content"],
-    "alert_emails": ["pinning@mozilla.org"],
-    "expires_in_version": "never",
-    "kind": "boolean",
-    "description": "Certificate pinning results for Mozilla sites (0 = failure, 1 = success)"
-  },
-  "CERT_PINNING_MOZ_TEST_RESULTS": {
-    "record_in_processes": ["main", "content"],
-    "alert_emails": ["pinning@mozilla.org"],
-    "expires_in_version": "never",
-    "kind": "boolean",
-    "description": "Certificate pinning test results for Mozilla sites (0 = failure, 1 = success)"
-  },
   "CERT_PINNING_MOZ_RESULTS_BY_HOST": {
     "record_in_processes": ["main", "content"],
     "alert_emails": ["dkeeler@mozilla.com", "pinning@mozilla.org"],
     "releaseChannelCollection": "opt-out",
     "bug_numbers": [1007844, 1521940],
     "expires_in_version": "never",
     "kind": "enumerated",
     "n_values": 512,
--- a/toolkit/components/telemetry/histogram-whitelists.json
+++ b/toolkit/components/telemetry/histogram-whitelists.json
@@ -544,18 +544,16 @@
     "CACHE_SERVICE_LOCK_WAIT_MAINTHREAD_NSSETDISKSMARTSIZECALLBACK_NOTIFY",
     "CANVAS_2D_USED",
     "CANVAS_WEBGL_USED",
     "CERT_CHAIN_KEY_SIZE_STATUS",
     "CERT_CHAIN_SHA1_POLICY_STATUS",
     "CERT_OCSP_ENABLED",
     "CERT_OCSP_REQUIRED",
     "CERT_PINNING_FAILURES_BY_CA",
-    "CERT_PINNING_MOZ_RESULTS",
-    "CERT_PINNING_MOZ_TEST_RESULTS",
     "CERT_PINNING_RESULTS",
     "CERT_PINNING_TEST_RESULTS",
     "CERT_VALIDATION_HTTP_REQUEST_CANCELED_TIME",
     "CERT_VALIDATION_HTTP_REQUEST_FAILED_TIME",
     "CERT_VALIDATION_HTTP_REQUEST_RESULT",
     "CERT_VALIDATION_HTTP_REQUEST_SUCCEEDED_TIME",
     "CHANGES_OF_DETECTED_LANGUAGE",
     "CHANGES_OF_TARGET_LANGUAGE",