Backed out changeset 674770e65a4f (bug 1265472) for memory leaks in browser_webauthn_telemetry.js
authorCarsten "Tomcat" Book <cbook@mozilla.com>
Wed, 26 Jul 2017 07:56:38 +0200
changeset 419684 4f821dab4306fd0aff947ce55a1304acd7e4cf95
parent 419683 f61a06a600af01916c79db690f129d809313bba4
child 419742 e8400551c2e39f24c75a009ebed496c7acd7bf47
child 419763 953c82d8136063c13f554a5ace23de69da942e72
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1265472
milestone56.0a1
backs out674770e65a4fc1f0f11c4d0d101625353c38ef60
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
Backed out changeset 674770e65a4f (bug 1265472) for memory leaks in browser_webauthn_telemetry.js
dom/webauthn/U2FTokenManager.cpp
dom/webauthn/moz.build
dom/webauthn/tests/browser/browser.ini
dom/webauthn/tests/browser/browser_webauthn_telemetry.js
dom/webauthn/tests/browser/frame_webauthn_success.html
dom/webauthn/tests/mochitest.ini
toolkit/components/telemetry/Histograms.json
toolkit/components/telemetry/Scalars.yaml
--- a/dom/webauthn/U2FTokenManager.cpp
+++ b/dom/webauthn/U2FTokenManager.cpp
@@ -247,40 +247,30 @@ U2FTokenManager::Register(WebAuthnTransa
   // UnknownError and terminate the operation.
 
   if ((aTransactionInfo.RpIdHash().Length() != SHA256_LENGTH) ||
       (aTransactionInfo.ClientDataHash().Length() != SHA256_LENGTH)) {
     AbortTransaction(NS_ERROR_DOM_UNKNOWN_ERR);
     return;
   }
 
-  mozilla::TimeStamp startTime = mozilla::TimeStamp::Now();
   mRegisterPromise = mTokenManagerImpl->Register(aTransactionInfo.Descriptors(),
                                                  aTransactionInfo.RpIdHash(),
                                                  aTransactionInfo.ClientDataHash(),
                                                  aTransactionInfo.TimeoutMS());
 
   mRegisterPromise->Then(GetCurrentThreadSerialEventTarget(), __func__,
-                         [tid, startTime](U2FRegisterResult&& aResult) {
+                         [tid](U2FRegisterResult&& aResult) {
                            U2FTokenManager* mgr = U2FTokenManager::Get();
                            mgr->MaybeConfirmRegister(tid, aResult);
-                           Telemetry::ScalarAdd(
-                             Telemetry::ScalarID::SECURITY_WEBAUTHN_USED,
-                             NS_LITERAL_STRING("U2FRegisterFinish"), 1);
-                           Telemetry::AccumulateTimeDelta(
-                             Telemetry::WEBAUTHN_CREATE_CREDENTIAL_MS,
-                             startTime);
                          },
                          [tid](nsresult rv) {
                            MOZ_ASSERT(NS_FAILED(rv));
                            U2FTokenManager* mgr = U2FTokenManager::Get();
                            mgr->MaybeAbortTransaction(tid, rv);
-                           Telemetry::ScalarAdd(
-                             Telemetry::ScalarID::SECURITY_WEBAUTHN_USED,
-                             NS_LITERAL_STRING("U2FRegisterAbort"), 1);
                          });
 }
 
 void
 U2FTokenManager::MaybeConfirmRegister(uint64_t aTransactionId,
                                       U2FRegisterResult& aResult)
 {
   if (mTransactionId != aTransactionId) {
@@ -311,40 +301,30 @@ U2FTokenManager::Sign(WebAuthnTransactio
   }
 
   if ((aTransactionInfo.RpIdHash().Length() != SHA256_LENGTH) ||
       (aTransactionInfo.ClientDataHash().Length() != SHA256_LENGTH)) {
     AbortTransaction(NS_ERROR_DOM_UNKNOWN_ERR);
     return;
   }
 
-  mozilla::TimeStamp startTime = mozilla::TimeStamp::Now();
   mSignPromise = mTokenManagerImpl->Sign(aTransactionInfo.Descriptors(),
                                          aTransactionInfo.RpIdHash(),
                                          aTransactionInfo.ClientDataHash(),
                                          aTransactionInfo.TimeoutMS());
 
   mSignPromise->Then(GetCurrentThreadSerialEventTarget(), __func__,
-                     [tid, startTime](U2FSignResult&& aResult) {
+                     [tid](U2FSignResult&& aResult) {
                        U2FTokenManager* mgr = U2FTokenManager::Get();
                        mgr->MaybeConfirmSign(tid, aResult);
-                       Telemetry::ScalarAdd(
-                         Telemetry::ScalarID::SECURITY_WEBAUTHN_USED,
-                         NS_LITERAL_STRING("U2FSignFinish"), 1);
-                       Telemetry::AccumulateTimeDelta(
-                         Telemetry::WEBAUTHN_GET_ASSERTION_MS,
-                         startTime);
                      },
                      [tid](nsresult rv) {
                        MOZ_ASSERT(NS_FAILED(rv));
                        U2FTokenManager* mgr = U2FTokenManager::Get();
                        mgr->MaybeAbortTransaction(tid, rv);
-                       Telemetry::ScalarAdd(
-                         Telemetry::ScalarID::SECURITY_WEBAUTHN_USED,
-                         NS_LITERAL_STRING("U2FSignAbort"), 1);
                      });
 }
 
 void
 U2FTokenManager::MaybeConfirmSign(uint64_t aTransactionId,
                                   U2FSignResult& aResult)
 {
   if (mTransactionId != aTransactionId) {
--- a/dom/webauthn/moz.build
+++ b/dom/webauthn/moz.build
@@ -55,9 +55,8 @@ LOCAL_INCLUDES += [
     '/dom/base',
     '/dom/crypto',
     '/security/manager/ssl',
     '/security/pkix/include',
     '/security/pkix/lib',
 ]
 
 MOCHITEST_MANIFESTS += ['tests/mochitest.ini']
-BROWSER_CHROME_MANIFESTS += ['tests/browser/browser.ini']
deleted file mode 100644
--- a/dom/webauthn/tests/browser/browser.ini
+++ /dev/null
@@ -1,9 +0,0 @@
-[DEFAULT]
-support-files =
-  frame_webauthn_success.html
-  ../cbor/*
-  ../pkijs/*
-  ../u2futil.js
-skip-if = !e10s
-
-[browser_webauthn_telemetry.js]
deleted file mode 100644
--- a/dom/webauthn/tests/browser/browser_webauthn_telemetry.js
+++ /dev/null
@@ -1,109 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this file,
- * You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-"use strict";
-
-// Return the scalars from the parent-process.
-function getParentProcessScalars(aChannel, aKeyed = false, aClear = false) {
-  const scalars = aKeyed ?
-    Services.telemetry.snapshotKeyedScalars(aChannel, aClear)["parent"] :
-    Services.telemetry.snapshotScalars(aChannel, aClear)["parent"];
-  return scalars || {};
-}
-
-function getTelemetryForScalar(aName) {
-  let scalars = getParentProcessScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTOUT, true);
-  return scalars[aName] || 0;
-}
-
-function cleanupTelemetry() {
-  Services.telemetry.clearScalars();
-  Services.telemetry.clearEvents();
-  Services.telemetry.getHistogramById("WEBAUTHN_CREATE_CREDENTIAL_MS").clear();
-  Services.telemetry.getHistogramById("WEBAUTHN_GET_ASSERTION_MS").clear();
-}
-
-function validateHistogramEntryCount(aHistogramName, aExpectedCount) {
-  let hist = Services.telemetry.getHistogramById(aHistogramName);
-  let resultIndexes = hist.snapshot();
-
-  let entriesSeen = 0;
-  for (let i = 0; i < resultIndexes.counts.length; i++) {
-    entriesSeen += resultIndexes.counts[i];
-  }
-
-  is(entriesSeen, aExpectedCount, "Expecting " + aExpectedCount + " histogram entries in " +
-     aHistogramName);
-}
-
-// Loads a page, and expects to have an element ID "result" added to the DOM when the page is done.
-async function executeTestPage(aUri) {
-  let win = await BrowserTestUtils.openNewBrowserWindow();
-  try {
-    await BrowserTestUtils.loadURI(win.gBrowser.selectedBrowser, aUri);
-
-    await ContentTask.spawn(win.gBrowser.selectedBrowser, null, async function() {
-      let condition = () => content.document.getElementById("result");
-      await ContentTaskUtils.waitForCondition(condition,
-                                              "Waited too long for operation to finish on "
-                                              + content.document.location);
-    });
-  } catch(e) {
-    ok(false, "Exception thrown executing test page: " + e);
-  } finally {
-    // Remove all the extra windows and tabs.
-    return BrowserTestUtils.closeWindow(win);
-  }
-}
-
-add_task(async function test_loopback() {
-  // These tests can't run simultaneously as the preference changes will race.
-  // So let's run them sequentially here, but in an async function so we can
-  // use await.
-  const testPage = "https://example.com/browser/dom/webauthn/tests/browser/frame_webauthn_success.html";
-  {
-    cleanupTelemetry();
-    // Enable the soft token, and execute a simple end-to-end test
-    Services.prefs.setBoolPref("security.webauth.webauthn", true);
-    Services.prefs.setBoolPref("security.webauth.webauthn_enable_softtoken", true);
-    Services.prefs.setBoolPref("security.webauth.webauthn_enable_usbtoken", false);
-
-    await executeTestPage(testPage);
-
-    let webauthn_used = getTelemetryForScalar("security.webauthn_used");
-    ok(webauthn_used, "Scalar keys are set: " + Object.keys(webauthn_used).join(", "));
-    is(webauthn_used["U2FRegisterFinish"], 1, "webauthn_used U2FRegisterFinish scalar should be 1");
-    is(webauthn_used["U2FSignFinish"], 1, "webauthn_used U2FSignFinish scalar should be 1");
-    is(webauthn_used["U2FSignAbort"], undefined, "webauthn_used U2FSignAbort scalar must be unset");
-    is(webauthn_used["U2FRegisterAbort"], undefined, "webauthn_used U2FRegisterAbort scalar must be unset");
-
-    validateHistogramEntryCount("WEBAUTHN_CREATE_CREDENTIAL_MS", 1);
-    validateHistogramEntryCount("WEBAUTHN_GET_ASSERTION_MS", 1);
-  }
-
-  {
-    cleanupTelemetry();
-    // Same as test_successful_loopback, but we will swap to using a (non-existent)
-    // usb token. This will cause U2FRegisterAbort to fire, but will not execute the
-    // Sign function, and no histogram entries will log.
-    Services.prefs.setBoolPref("security.webauth.webauthn", true);
-    Services.prefs.setBoolPref("security.webauth.webauthn_enable_softtoken", false);
-    Services.prefs.setBoolPref("security.webauth.webauthn_enable_usbtoken", true);
-
-    await executeTestPage(testPage);
-
-    let webauthn_used = getTelemetryForScalar("security.webauthn_used");
-    ok(webauthn_used, "Scalar keys are set: " + Object.keys(webauthn_used).join(", "));
-    is(webauthn_used["U2FRegisterFinish"], undefined, "webauthn_used U2FRegisterFinish must be unset");
-    is(webauthn_used["U2FSignFinish"], undefined, "webauthn_used U2FSignFinish scalar must be unset");
-    is(webauthn_used["U2FRegisterAbort"], 1, "webauthn_used U2FRegisterAbort scalar should be a 1");
-    is(webauthn_used["U2FSignAbort"], undefined, "webauthn_used U2FSignAbort scalar must be unset");
-
-    validateHistogramEntryCount("WEBAUTHN_CREATE_CREDENTIAL_MS", 0);
-    validateHistogramEntryCount("WEBAUTHN_GET_ASSERTION_MS", 0);
-  }
-
-  // There aren't tests for register succeeding and sign failing, as I don't see an easy way to prompt
-  // the soft token to fail that way _and_ trigger the Abort telemetry.
-});
deleted file mode 100644
--- a/dom/webauthn/tests/browser/frame_webauthn_success.html
+++ /dev/null
@@ -1,128 +0,0 @@
-<!DOCTYPE html>
-<meta charset=utf-8>
-<head>
-  <title>Full-run test for MakeCredential/GetAssertion for W3C Web Authentication</title>
-  <script type="text/javascript" src="u2futil.js"></script>
-  <script type="text/javascript" src="../pkijs/common.js"></script>
-  <script type="text/javascript" src="../pkijs/asn1.js"></script>
-  <script type="text/javascript" src="../pkijs/x509_schema.js"></script>
-  <script type="text/javascript" src="../pkijs/x509_simpl.js"></script>
-  <script type="text/javascript" src="../cbor/cbor.js"></script>
-  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
-</head>
-<body>
-
-<h1>Full-run test for MakeCredential/GetAssertion for W3C Web Authentication</h1>
-<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1265472">Mozilla Bug 1265472</a>
-
-<script class="testbody" type="text/javascript">
-"use strict";
-
-let gCredentialChallenge = new Uint8Array(16);
-window.crypto.getRandomValues(gCredentialChallenge);
-let gAssertionChallenge = new Uint8Array(16);
-window.crypto.getRandomValues(gAssertionChallenge);
-
-// the WebAuthn browser chrome tests watch for an element 'result' to appear in the DOM
-function signalCompletion(aText) {
-  console.log(aText)
-  let result = document.createElement('h1');
-  result.id = "result";
-  result.textContent = aText;
-  document.body.append(result);
-}
-
-function util_decodeCreatedCredential(aCredInfo) {
-  /* PublicKeyCredential : Credential
-     - rawId: Key Handle buffer pulled from U2F Register() Response
-     - response : AuthenticatorAttestationResponse : AuthenticatorResponse
-       - attestationObject: CBOR object
-       - clientDataJSON: serialized JSON
-     - clientExtensionResults: (not yet supported)
-  */
-
-  return webAuthnDecodeAttestation(aCredInfo.response.attestationObject.buffer)
-  .then(function(decodedResult) {
-    aCredInfo.publicKeyHandle = decodedResult.publicKeyHandle;
-    aCredInfo.attestationObject = decodedResult.attestationObject;
-    return aCredInfo;
-  });
-}
-
-function util_checkAssertionAndSigValid(aPublicKey, aAssertion) {
-  /* PublicKeyCredential : Credential
-     - rawId: ID of Credential from AllowList that succeeded
-     - response : AuthenticatorAssertionResponse : AuthenticatorResponse
-       - clientDataJSON: serialized JSON
-       - authenticatorData: RP ID Hash || U2F Sign() Response
-       - signature: U2F Sign() Response
-  */
-
-  // Parse the signature data
-  if (aAssertion.response.signature[0] != 0x01) {
-    throw "User presence byte not set";
-  }
-
-  let presenceAndCounter = aAssertion.response.signature.slice(0,5);
-  let signatureValue = aAssertion.response.signature.slice(5);
-
-  let rpIdHash = aAssertion.response.authenticatorData.slice(0,32);
-
-  // Assemble the signed data and verify the signature
-  return deriveAppAndChallengeParam(window.location.host, aAssertion.response.clientDataJSON)
-  .then(function(aParams) {
-    console.log(aParams.appParam, rpIdHash, presenceAndCounter, aParams.challengeParam);
-    console.log("ClientData buffer: ", hexEncode(aAssertion.response.clientDataJSON));
-    console.log("ClientDataHash: ", hexEncode(aParams.challengeParam));
-    return assembleSignedData(aParams.appParam, presenceAndCounter, aParams.challengeParam);
-  })
-  .then(function(aSignedData) {
-    console.log(aPublicKey, aSignedData, signatureValue);
-    return verifySignature(aPublicKey, aSignedData, signatureValue);
-  })
-}
-
-let rp = {id: document.domain, name: "none", icon: "none"};
-let user = {id: "none", name: "none", icon: "none", displayName: "none"};
-let param = {type: "public-key", algorithm: "ES256"};
-let makeCredentialOptions = {
-  rp: rp,
-  user: user,
-  challenge: gCredentialChallenge,
-  parameters: [param]
-};
-
-let credm = navigator.credentials;
-credm.create({publicKey: makeCredentialOptions})
-.then(util_decodeCreatedCredential)
-.then(function testAssertion(aCredInfo) {
-  let newCredential = {
-    type: "public-key",
-    id: Uint8Array.from(aCredInfo.rawId),
-    transports: ["usb"],
-  }
-
-  let publicKeyCredentialRequestOptions = {
-    challenge: gAssertionChallenge,
-    timeout: 5000, // the minimum timeout is actually 15 seconds
-    rpId: document.domain,
-    allowList: [newCredential]
-  };
-
-  return credm.get({publicKey: publicKeyCredentialRequestOptions})
-  .then(function(aAssertion) {
-    /* Pass along the pubKey. */
-    return util_checkAssertionAndSigValid(aCredInfo.publicKeyHandle, aAssertion);
-  });
-})
-.then(function(aSigVerifyResult) {
-  signalCompletion("Signing signature verified: " + aSigVerifyResult);
-})
-.catch(function failure(aReason) {
-  signalCompletion("Failure: " + aReason);
-});
-
-</script>
-
-</body>
-</html>
\ No newline at end of file
--- a/dom/webauthn/tests/mochitest.ini
+++ b/dom/webauthn/tests/mochitest.ini
@@ -1,21 +1,24 @@
 [DEFAULT]
 support-files =
-  cbor/*
-  pkijs/*
+  cbor/cbor.js
+  pkijs/asn1.js
+  pkijs/common.js
+  pkijs/x509_schema.js
+  pkijs/x509_simpl.js
   u2futil.js
 
 [test_webauthn_loopback.html]
 skip-if = !e10s
 scheme = https
 [test_webauthn_no_token.html]
 skip-if = !e10s
 scheme = https
 [test_webauthn_make_credential.html]
 skip-if = !e10s
 scheme = https
 [test_webauthn_get_assertion.html]
 skip-if = !e10s
 scheme = https
 [test_webauthn_sameorigin.html]
 skip-if = !e10s
-scheme = https
+scheme = https
\ No newline at end of file
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -13608,32 +13608,10 @@
     "alert_emails": ["alwu@mozilla.com"],
     "expires_in_version": "60",
     "kind": "exponential",
     "high": 10000,
     "n_buckets": 100,
     "bug_numbers": [1274919],
     "description": "Measure the time how long the cursor is hovering before opening the unselcted tab. Only record the data if someone requests for sending unselected tab hover msg.",
     "releaseChannelCollection": "opt-out"
-  },
-  "WEBAUTHN_CREATE_CREDENTIAL_MS": {
-    "record_in_processes": ["main"],
-    "alert_emails": ["seceng-telemetry@mozilla.com", "jjones@mozilla.com"],
-    "expires_in_version": "70",
-    "releaseChannelCollection": "opt-out",
-    "kind": "exponential",
-    "high": 120000,
-    "n_buckets": 100,
-    "bug_numbers": [1265472],
-    "description": "Time in milliseconds to complete a WebAuthn create credential."
-  },
-  "WEBAUTHN_GET_ASSERTION_MS": {
-    "record_in_processes": ["main"],
-    "alert_emails": ["seceng-telemetry@mozilla.com", "jjones@mozilla.com"],
-    "expires_in_version": "70",
-    "releaseChannelCollection": "opt-out",
-    "kind": "exponential",
-    "high": 120000,
-    "n_buckets": 100,
-    "bug_numbers": [1265472],
-    "description": "Time in milliseconds to complete a WebAuthn get assertion."
   }
 }
--- a/toolkit/components/telemetry/Scalars.yaml
+++ b/toolkit/components/telemetry/Scalars.yaml
@@ -354,32 +354,16 @@ security:
     expires: "62"
     kind: boolean
     keyed: true
     notification_emails:
       - seceng-telemetry@mozilla.com
     release_channel_collection: opt-out
     record_in_processes:
       - main
-  webauthn_used:
-    bug_numbers:
-      - 1265472
-    description: >
-      Counts of how often Web Authentication was used in this session, keyed
-      by authenticator protocol, method and result. Currently: U2FRegisterFinish,
-      U2FRegisterAbort, U2FSignFinish, U2FSignAbort.
-    expires: "70"
-    kind: uint
-    keyed: true
-    notification_emails:
-      - seceng-telemetry@mozilla.com
-      - jjones@mozilla.com
-    release_channel_collection: opt-out
-    record_in_processes:
-      - main
 
 preferences:
   created_new_user_prefs_file:
     bug_numbers:
       - 1367813
     description: >-
       A boolean indicating that profile/prefs.js was not found and it is being
       created for the first time in this session.