Bug 1322611 - After a crash compute the SHA256 hash of a minidump and attach it to the crash ping; r=Ted, nchen

This patch changes the crashreporter client code as well as the crash service
code to compute a SHA256 hash of a crash' minidump file and add it to the
crash ping. The crash service code computes the hash on the fly before handing
over the crash to the crash manager; the crash manager will then add it to the
crash ping. The crashreporter client on the other hand sends the hash via the
ping it generates but it also adds it to the event file so that the crash
manager can pick it up and send it along with its own crash ping. On Fennec
the crashreporter activity takes care of computing the hash.

SHA256 hash computation uses nsICryptoHash in the crash service, the
java.security.MessageDigest class in Fennec, the bundled NSS library in the
crashreporter when running on Windows and Mac and the system-provided NSS
library under Linux. The latter is required because the crashreporter client
uses the system curl library which is linked to NSS and which would thus clash
with the bundled one if used together.

This patch introduces two new methods for the nsICrashService interface:
|getMinidumpForID()| and |getExtraFileForID()|, these reliably retrieve the
.dmp and .extra files associated with a crash and ensure the files exist
before returning. These new methods are used in the CrashService for
processing and will become the only way to reliably retrieve those files
from a crash ID.

MozReview-Commit-ID: 8BKvqj6URcO

--HG--
extra : source : a4d8291c56fcde00238ab3166bbe6af6dd602340

dom/plugins/ipc/PluginModuleParent.cpp

@@ -1226,13 +1226,9 @@ PluginModuleChromeParent::TakeFullMinidump(base::ProcessId aContentPid,
     // hangs we take this earlier (see ProcessHangMonitor) from a background
     // thread. We do this before we message the main thread about the hang
     // since the posted message will trash our browser stack state.
-    bool exists;
     nsCOMPtr<nsIFile> browserDumpFile;
-    if (!aBrowserDumpId.IsEmpty() &&
+    if (CrashReporter::GetMinidumpForID(aBrowserDumpId,
-        CrashReporter::GetMinidumpForID(aBrowserDumpId, getter_AddRefs(browserDumpFile)) &&
+                                        getter_AddRefs(browserDumpFile))) {
-        browserDumpFile &&
-        NS_SUCCEEDED(browserDumpFile->Exists(&exists)) && exists)
-    {
         // We have a single browser report, generate a new plugin process parent
         // report and pair it up with the browser report handed in.
         reportsReady = mCrashReporter->GenerateMinidumpAndPair(
@@ -1264,8 +1260,7 @@ PluginModuleChromeParent::TakeFullMinidump(base::ProcessId aContentPid,
                  NS_ConvertUTF16toUTF8(aDumpId).get()));
         nsAutoCString additionalDumps("browser");
         nsCOMPtr<nsIFile> pluginDumpFile;
-        if (GetMinidumpForID(aDumpId, getter_AddRefs(pluginDumpFile)) &&
+        if (GetMinidumpForID(aDumpId, getter_AddRefs(pluginDumpFile))) {
-            pluginDumpFile) {
 #ifdef MOZ_CRASHREPORTER_INJECTOR
             // If we have handles to the flash sandbox processes on Windows,
             // include those minidumps as well.

mobile/android/base/java/org/mozilla/gecko/CrashReporter.java

@@ -12,6 +12,7 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.FileReader;
+import java.io.FileWriter;
 import java.io.InputStreamReader;
 import java.io.IOException;
 import java.io.OutputStream;
@@ -19,6 +20,7 @@ import java.net.HttpURLConnection;
 import java.net.URL;
 import java.nio.channels.Channels;
 import java.nio.channels.FileChannel;
+import java.security.MessageDigest;
 import java.util.zip.GZIPOutputStream;
 
 import org.mozilla.gecko.AppConstants.Versions;
@@ -138,6 +140,7 @@ public class CrashReporter extends AppCompatActivity
         mPendingExtrasFile = new File(pendingDir, extrasFile.getName());
         moveFile(extrasFile, mPendingExtrasFile);
 
+        computeMinidumpHash(mPendingExtrasFile, mPendingMinidumpFile);
         mExtrasStringMap = new HashMap<String, String>();
         readStringsFromFile(mPendingExtrasFile.getPath(), mExtrasStringMap);
 
@@ -267,6 +270,46 @@ public class CrashReporter extends AppCompatActivity
         backgroundSendReport();
     }
 
+    private void computeMinidumpHash(File extraFile, File minidump) {
+        try {
+            FileInputStream stream = new FileInputStream(minidump);
+            MessageDigest md = MessageDigest.getInstance("SHA-256");
+
+            try {
+                byte[] buffer = new byte[4096];
+                int readBytes;
+
+                while ((readBytes = stream.read(buffer)) != -1) {
+                    md.update(buffer, 0, readBytes);
+                }
+            } finally {
+              stream.close();
+            }
+
+            byte[] digest = md.digest();
+            StringBuilder hash = new StringBuilder(84);
+
+            hash.append("MinidumpSha256Hash=");
+
+            for (int i = 0; i < digest.length; i++) {
+              hash.append(Integer.toHexString((digest[i] & 0xf0) >> 4));
+              hash.append(Integer.toHexString(digest[i] & 0x0f));
+            }
+
+            hash.append('\n');
+
+            FileWriter writer = new FileWriter(extraFile, /* append */ true);
+
+            try {
+                writer.write(hash.toString());
+            } finally {
+                writer.close();
+            }
+        } catch (Exception e) {
+            Log.e(LOGTAG, "exception while computing the minidump hash: ", e);
+        }
+    }
+
     private boolean readStringsFromFile(String filePath, Map<String, String> stringMap) {
         try {
             BufferedReader reader = new BufferedReader(new FileReader(filePath));

toolkit/components/crashes/CrashManager.jsm

@@ -40,30 +40,44 @@ function dateToDays(date) {
 }
 
 /**
- * Parse the string stored in the specified field as JSON and then remove the
+ * Get a field from the specified object and remove it.
- * field from the object. The string might also be returned without parsing.
  *
  * @param obj {Object} The object holding the field
  * @param field {String} The name of the field to be parsed and removed
- * @param [parseAsJson=true] {Boolean} If true parse the field's contents as if
- *        it were JSON code, otherwise return the rew string.
  *
- * @returns {Object|String} the parsed object or the raw string
+ * @returns {String} the field contents as a string, null if none was found
  */
-function parseAndRemoveField(obj, field, parseAsJson = true) {
+function getAndRemoveField(obj, field) {
   let value = null;
 
   if (field in obj) {
-    if (!parseAsJson) {
+    // We split extra files on LF characters but Windows-generated ones might
-      // We split extra files on LF characters but Windows-generated ones might
+    // contain trailing CR characters so trim them here.
-      // contain trailing CR characters so trim them here.
+    value = obj[field].trim();
-      value = obj[field].trim();
+
-    } else {
+    delete obj[field];
-      try {
+  }
-        value = JSON.parse(obj[field]);
+
-      } catch (e) {
+  return value;
-        Cu.reportError(e);
+}
-      }
+
+/**
+ * Parse the string stored in the specified field as JSON and then remove the
+ * field from the object.
+ *
+ * @param obj {Object} The object holding the field
+ * @param field {String} The name of the field to be parsed and removed
+ *
+ * @returns {Object} the parsed object, null if none was found
+ */
+function parseAndRemoveField(obj, field) {
+  let value = null;
+
+  if (field in obj) {
+    try {
+      value = JSON.parse(obj[field]);
+    } catch (e) {
+      Cu.reportError(e);
     }
 
     delete obj[field];
@@ -459,7 +473,7 @@ this.CrashManager.prototype = Object.freeze({
       if (processType === this.PROCESS_TYPE_CONTENT) {
         this._sendCrashPing(id, processType, date, metadata);
       }
-   }.bind(this));
+    }.bind(this));
 
     return promise;
   },
@@ -637,12 +651,12 @@ this.CrashManager.prototype = Object.freeze({
     let reportMeta = Cu.cloneInto(metadata, myScope);
     let crashEnvironment = parseAndRemoveField(reportMeta,
                                                "TelemetryEnvironment");
-    let sessionId = parseAndRemoveField(reportMeta, "TelemetrySessionId",
+    let sessionId = getAndRemoveField(reportMeta, "TelemetrySessionId");
-                                        /* parseAsJson */ false);
     let stackTraces = parseAndRemoveField(reportMeta, "StackTraces");
+    let minidumpSha256Hash = getAndRemoveField(reportMeta,
+                                               "MinidumpSha256Hash");
     // If CrashPingUUID is not present then Telemetry will generate a new UUID
-    let pingId = parseAndRemoveField(reportMeta, "CrashPingUUID",
+    let pingId = getAndRemoveField(reportMeta, "CrashPingUUID");
-                                     /* parseAsJson */ false);
 
     // Filter the remaining annotations to remove privacy-sensitive ones
     reportMeta = this._filterAnnotations(reportMeta);
@@ -654,6 +668,7 @@ this.CrashManager.prototype = Object.freeze({
         crashTime: date.toISOString().slice(0, 13) + ":00:00.000Z", // per-hour resolution
         sessionId,
         crashId,
+        minidumpSha256Hash,
         processType: type,
         stackTraces,
         metadata: reportMeta,

toolkit/components/crashes/CrashService.js

@@ -14,6 +14,44 @@ Cu.import("resource://gre/modules/Services.jsm", this);
 Cu.import("resource://gre/modules/Task.jsm", this);
 Cu.import("resource://gre/modules/XPCOMUtils.jsm", this);
 
+/**
+ * Computes the SHA256 hash of the minidump file associated with a crash
+ *
+ * @param crashID {string} Crash ID. Likely a UUID.
+ *
+ * @returns {Promise} A promise that resolves to the hash value of the
+ *          minidump. If the hash could not be computed then null is returned
+ *          instead.
+ */
+function computeMinidumpHash(id) {
+  let cr = Cc["@mozilla.org/toolkit/crash-reporter;1"]
+             .getService(Components.interfaces.nsICrashReporter);
+
+  return Task.spawn(function* () {
+    try {
+      let minidumpFile = cr.getMinidumpForID(id);
+      let minidumpData = yield OS.File.read(minidumpFile.path);
+      let hasher = Cc["@mozilla.org/security/hash;1"]
+                     .createInstance(Ci.nsICryptoHash);
+      hasher.init(hasher.SHA256);
+      hasher.update(minidumpData, minidumpData.length);
+
+      let hashBin = hasher.finish(false);
+      let hash = "";
+
+      for (let i = 0; i < hashBin.length; i++) {
+        // Every character in the hash string contains a byte of the hash data
+        hash += ("0" + hashBin.charCodeAt(i).toString(16)).slice(-2);
+      }
+
+      return hash;
+    } catch (e) {
+      Cu.reportError(e);
+      return null;
+    }
+  });
+}
+
 /**
  * Process the .extra file associated with the crash id and return the
  * annotations it contains in an object.
@@ -26,20 +64,18 @@ Cu.import("resource://gre/modules/XPCOMUtils.jsm", this);
 function processExtraFile(id) {
   let cr = Cc["@mozilla.org/toolkit/crash-reporter;1"]
              .getService(Components.interfaces.nsICrashReporter);
-  let extraPath = OS.Path.join(cr.minidumpPath.path, id + ".extra");
 
   return Task.spawn(function* () {
     try {
+      let extraFile = cr.getExtraFileForID(id);
       let decoder = new TextDecoder();
-      let extraFile = yield OS.File.read(extraPath);
+      let extraData = yield OS.File.read(extraFile.path);
-      let extraData = decoder.decode(extraFile);
 
-      return parseKeyValuePairs(extraData);
+      return parseKeyValuePairs(decoder.decode(extraData));
     } catch (e) {
       Cu.reportError(e);
+      return {};
     }
-
-    return {};
   });
 }
 
@@ -89,13 +125,23 @@ CrashService.prototype = Object.freeze({
       throw new Error("Unrecognized CRASH_TYPE: " + crashType);
     }
 
+    let blocker = Task.spawn(function* () {
+      let metadata = yield processExtraFile(id);
+      let hash = yield computeMinidumpHash(id);
+
+      if (hash) {
+        metadata.MinidumpSha256Hash = hash;
+      }
+
+      yield Services.crashmanager.addCrash(processType, crashType, id,
+                                           new Date(), metadata);
+    });
+
     AsyncShutdown.profileBeforeChange.addBlocker(
-      "CrashService waiting for content crash ping to be sent",
+      "CrashService waiting for content crash ping to be sent", blocker
-      processExtraFile(id).then(metadata => {
-        return Services.crashmanager.addCrash(processType, crashType, id,
-                                              new Date(), metadata)
-      })
     );
+
+    blocker.then(AsyncShutdown.profileBeforeChange.removeBlocker(blocker));
   },
 
   observe(subject, topic, data) {

toolkit/components/crashes/nsICrashService.idl

@@ -16,6 +16,8 @@ interface nsICrashService : nsISupports
    *        One of the CRASH_TYPE constants defined below.
    * @param id
    *        Crash ID. Likely a UUID.
+   *
+   * @return {Promise} A promise that resolves after the crash has been stored
    */
   void addCrash(in long processType, in long crashType, in AString id);
 

toolkit/components/crashes/tests/xpcshell/test_crash_manager.js

@@ -210,8 +210,11 @@ add_task(function* test_schedule_maintenance() {
 });
 
 const crashId = "3cb67eba-0dc7-6f78-6a569a0e-172287ec";
+const crashPingUuid = "103dbdf2-339b-4b9c-a7cc-5f9506ea9d08";
 const productName = "Firefox";
 const productId = "{ec8030f7-c20a-464f-9b0e-13a3a9e97384}";
+const sha256Hash =
+  "f8410c3ac4496cfa9191a1240f0e365101aef40c7bf34fc5bcb8ec511832ed79";
 const stackTraces = "{\"status\":\"OK\"}";
 
 add_task(function* test_main_crash_event_file() {
@@ -230,6 +233,7 @@ add_task(function* test_main_crash_event_file() {
     "ProductID=" + productId + "\n" +
     "TelemetryEnvironment=" + JSON.stringify(theEnvironment) + "\n" +
     "TelemetrySessionId=" + sessionId + "\n" +
+    "MinidumpSha256Hash=" + sha256Hash + "\n" +
     "StackTraces=" + stackTraces + "\n" +
     "ThisShouldNot=end-up-in-the-ping\n";
 
@@ -244,7 +248,7 @@ add_task(function* test_main_crash_event_file() {
   Assert.equal(crashes[0].metadata.ProductName, productName);
   Assert.equal(crashes[0].metadata.ProductID, productId);
   Assert.ok(crashes[0].metadata.TelemetryEnvironment);
-  Assert.equal(Object.getOwnPropertyNames(crashes[0].metadata).length, 6);
+  Assert.equal(Object.getOwnPropertyNames(crashes[0].metadata).length, 7);
   Assert.equal(crashes[0].metadata.TelemetrySessionId, sessionId);
   Assert.ok(crashes[0].metadata.StackTraces);
   Assert.deepEqual(crashes[0].crashDate, DUMMY_DATE);
@@ -253,6 +257,7 @@ add_task(function* test_main_crash_event_file() {
     [["payload", "hasCrashEnvironment"], true],
     [["payload", "metadata", "ProductName"], productName],
     [["payload", "metadata", "ProductID"], productId],
+    [["payload", "minidumpSha256Hash"], sha256Hash],
     [["payload", "crashId"], crashId],
     [["payload", "stackTraces", "status"], "OK"],
     [["payload", "sessionId"], sessionId],
@@ -301,6 +306,45 @@ add_task(function* test_main_crash_event_file_noenv() {
   Assert.equal(count, 0);
 });
 
+add_task(function* test_main_crash_event_file_override_ping_uuid() {
+  let ac = new TelemetryArchiveTesting.Checker();
+  yield ac.promiseInit();
+
+  let m = yield getManager();
+  const fileContent = crashId + "\n" +
+    "ProductName=" + productName + "\n" +
+    "ProductID=" + productId + "\n" +
+    "CrashPingUUID=" + crashPingUuid + "\n";
+
+  yield m.createEventsFile(crashId, "crash.main.2", DUMMY_DATE, fileContent);
+  let count = yield m.aggregateEventsFiles();
+  Assert.equal(count, 1);
+
+  let crashes = yield m.getCrashes();
+  Assert.equal(crashes.length, 1);
+  Assert.equal(crashes[0].id, crashId);
+  Assert.equal(crashes[0].type, "main-crash");
+  Assert.deepEqual(crashes[0].metadata, {
+    CrashPingUUID: crashPingUuid,
+    ProductName: productName,
+    ProductID: productId
+  });
+  Assert.deepEqual(crashes[0].crashDate, DUMMY_DATE);
+
+  let found = yield ac.promiseFindPing("crash", [
+    [["id"], crashPingUuid],
+    [["payload", "hasCrashEnvironment"], false],
+    [["payload", "metadata", "ProductName"], productName],
+    [["payload", "metadata", "ProductID"], productId],
+  ]);
+  Assert.ok(found, "Telemetry ping submitted for found crash");
+  Assert.equal(found.payload.metadata.CrashPingUUID, undefined,
+               "Crash ping UUID should be filtered out");
+
+  count = yield m.aggregateEventsFiles();
+  Assert.equal(count, 0);
+});
+
 add_task(function* test_crash_submission_event_file() {
   let m = yield getManager();
   yield m.createEventsFile("1", "crash.main.2", DUMMY_DATE, "crash1");
@@ -466,12 +510,14 @@ add_task(function* test_content_crash_ping() {
   let id = yield m.createDummyDump();
   yield m.addCrash(m.PROCESS_TYPE_CONTENT, m.CRASH_TYPE_CRASH, id, DUMMY_DATE, {
     StackTraces: stackTraces,
+    MinidumpSha256Hash: sha256Hash,
     ThisShouldNot: "end-up-in-the-ping"
   });
   yield m._pingPromise;
 
   let found = yield ac.promiseFindPing("crash", [
     [["payload", "crashId"], id],
+    [["payload", "minidumpSha256Hash"], sha256Hash],
     [["payload", "processType"], m.PROCESS_TYPE_CONTENT],
     [["payload", "stackTraces", "status"], "OK"],
   ]);

toolkit/components/telemetry/docs/data/crash-ping.rst

@@ -37,6 +37,7 @@ Structure:
                            // in startup. Added in Firefox 48 with the
                            // intention of uplifting to Firefox 46
         crashId: <UUID>, // Optional, ID of the associated crash
+        minidumpSha256Hash: <hash>, // SHA256 hash of the minidump file
         stackTraces: { ... }, // Optional, see below
         metadata: { // Annotations saved while Firefox was running. See nsExceptionHandler.cpp for more information
           ProductID: "{ec8030f7-c20a-464f-9b0e-13a3a9e97384}",

toolkit/crashreporter/client/crashreporter.cpp

@@ -11,6 +11,7 @@
 #endif
 
 #include <fstream>
+#include <iomanip>
 #include <sstream>
 #include <memory>
 #include <ctime>
@@ -18,6 +19,13 @@
 #include <cstring>
 #include <string>
 
+#ifdef XP_LINUX
+#include <dlfcn.h>
+#endif
+
+#include "nss.h"
+#include "sechash.h"
+
 using std::string;
 using std::istream;
 using std::ifstream;
@@ -436,6 +444,117 @@ bool ShouldEnableSending()
   return ((rand() % 100) < MOZ_CRASHREPORTER_ENABLE_PERCENT);
 }
 
+static string ComputeDumpHash() {
+#ifdef XP_LINUX
+  // On Linux we rely on the system-provided libcurl which uses nss so we have
+  // to also use the system-provided nss instead of the ones we have bundled.
+  const char* libnssNames[] = {
+    "libnss3.so",
+#ifndef HAVE_64BIT_BUILD
+    // 32-bit versions on 64-bit hosts
+    "/usr/lib32/libnss3.so",
+#endif
+  };
+  void* lib = nullptr;
+
+  for (const char* libname : libnssNames) {
+    lib = dlopen(libname, RTLD_NOW);
+
+    if (lib) {
+      break;
+    }
+  }
+
+  if (!lib) {
+    return "";
+  }
+
+  SECStatus (*NSS_Initialize)(const char*, const char*, const char*,
+                              const char*, PRUint32);
+  HASHContext* (*HASH_Create)(HASH_HashType);
+  void (*HASH_Destroy)(HASHContext*);
+  void (*HASH_Begin)(HASHContext*);
+  void (*HASH_Update)(HASHContext*, const unsigned char*, unsigned int);
+  void (*HASH_End)(HASHContext*, unsigned char*, unsigned int*, unsigned int);
+
+  *(void**) (&NSS_Initialize) = dlsym(lib, "NSS_Initialize");
+  *(void**) (&HASH_Create) = dlsym(lib, "HASH_Create");
+  *(void**) (&HASH_Destroy) = dlsym(lib, "HASH_Destroy");
+  *(void**) (&HASH_Begin) = dlsym(lib, "HASH_Begin");
+  *(void**) (&HASH_Update) = dlsym(lib, "HASH_Update");
+  *(void**) (&HASH_End) = dlsym(lib, "HASH_End");
+
+  if (!HASH_Create || !HASH_Destroy || !HASH_Begin || !HASH_Update ||
+      !HASH_End) {
+    return "";
+  }
+#endif
+  // Minimal NSS initialization so we can use the hash functions
+  const PRUint32 kNssFlags = NSS_INIT_READONLY | NSS_INIT_NOROOTINIT |
+                             NSS_INIT_NOMODDB | NSS_INIT_NOCERTDB;
+  if (NSS_Initialize(nullptr, "", "", "", kNssFlags) != SECSuccess) {
+    return "";
+  }
+
+  HASHContext* hashContext = HASH_Create(HASH_AlgSHA256);
+
+  if (!hashContext) {
+    return "";
+  }
+
+  HASH_Begin(hashContext);
+
+  ifstream* f = UIOpenRead(gReporterDumpFile, /* binary */ true);
+  bool error = false;
+
+  // Read the minidump contents
+  if (f->is_open()) {
+    uint8_t buff[4096];
+
+    do {
+      f->read((char*) buff, sizeof(buff));
+
+      if (f->bad()) {
+        error = true;
+        break;
+      }
+
+      HASH_Update(hashContext, buff, f->gcount());
+    } while (!f->eof());
+
+    f->close();
+  } else {
+    error = true;
+  }
+
+  delete f;
+
+  // Finalize the hash computation
+  uint8_t result[SHA256_LENGTH];
+  uint32_t resultLen = 0;
+
+  HASH_End(hashContext, result, &resultLen, SHA256_LENGTH);
+
+  if (resultLen != SHA256_LENGTH) {
+    error = true;
+  }
+
+  HASH_Destroy(hashContext);
+
+  if (!error) {
+    ostringstream hash;
+
+    for (size_t i = 0; i < SHA256_LENGTH; i++) {
+      hash << std::setw(2) << std::setfill('0') << std::hex
+           << static_cast<unsigned int>(result[i]);
+    }
+
+    return hash.str();
+  } else {
+    return ""; // If we encountered an error, return an empty hash
+  }
+}
+
 } // namespace CrashReporter
 
 using namespace CrashReporter;
@@ -638,9 +757,15 @@ int main(int argc, char** argv)
     }
 
     // Assemble and send the crash ping
+    string hash;
     string pingUuid;
 
-    if (SendCrashPing(queryParameters, pingUuid)) {
+    hash = ComputeDumpHash();
+    if (!hash.empty()) {
+      AppendToEventFile("MinidumpSha256Hash", hash);
+    }
+
+    if (SendCrashPing(queryParameters, hash, pingUuid)) {
       AppendToEventFile("CrashPingUUID", pingUuid);
     }
 

toolkit/crashreporter/client/crashreporter.h

@@ -118,7 +118,8 @@ namespace CrashReporter {
   bool ShouldEnableSending();
 
   // Telemetry ping
-  bool SendCrashPing(StringTable& strings, std::string& pingUuid);
+  bool SendCrashPing(StringTable& strings, const std::string& hash,
+                     std::string& pingUuid);
 
   static const unsigned int kSaveCount = 10;
 }
@@ -151,7 +152,7 @@ bool UIEnsurePathExists(const std::string& path);
 bool UIFileExists(const std::string& path);
 bool UIMoveFile(const std::string& oldfile, const std::string& newfile);
 bool UIDeleteFile(const std::string& oldfile);
-std::ifstream* UIOpenRead(const std::string& filename);
+std::ifstream* UIOpenRead(const std::string& filename, bool binary = false);
 std::ofstream* UIOpenWrite(const std::string& filename,
                            bool append=false,
                            bool binary=false);

toolkit/crashreporter/client/crashreporter_gtk_common.cpp

@@ -374,26 +374,6 @@ bool UIGetSettingsPath(const string& vendor,
   return true;
 }
 
-bool UIEnsurePathExists(const string& path)
-{
-  int ret = mkdir(path.c_str(), S_IRWXU);
-  int e = errno;
-  if (ret == -1 && e != EEXIST)
-    return false;
-
-  return true;
-}
-
-bool UIFileExists(const string& path)
-{
-  struct stat sb;
-  int ret = stat(path.c_str(), &sb);
-  if (ret == -1 || !(sb.st_mode & S_IFREG))
-    return false;
-
-  return true;
-}
-
 bool UIMoveFile(const string& file, const string& newfile)
 {
   if (!rename(file.c_str(), newfile.c_str()))
@@ -424,30 +404,3 @@ bool UIMoveFile(const string& file, const string& newfile)
   waitpid(pID, &status, 0);
   return UIFileExists(newfile);
 }
-
-bool UIDeleteFile(const string& file)
-{
-  return (unlink(file.c_str()) != -1);
-}
-
-std::ifstream* UIOpenRead(const string& filename)
-{
-  return new std::ifstream(filename.c_str(), std::ios::in);
-}
-
-std::ofstream* UIOpenWrite(const string& filename,
-                           bool append, // append=false
-                           bool binary) // binary=false
-{
-  std::ios_base::openmode mode = std::ios::out;
-
-  if (append) {
-    mode = mode | std::ios::app;
-  }
-
-  if (binary) {
-    mode = mode | std::ios::binary;
-  }
-
-  return new std::ofstream(filename.c_str(), mode);
-}

toolkit/crashreporter/client/crashreporter_osx.mm

@@ -857,26 +857,6 @@ bool UIGetSettingsPath(const string& vendor,
   return true;
 }
 
-bool UIEnsurePathExists(const string& path)
-{
-  int ret = mkdir(path.c_str(), S_IRWXU);
-  int e = errno;
-  if (ret == -1 && e != EEXIST)
-    return false;
-
-  return true;
-}
-
-bool UIFileExists(const string& path)
-{
-  struct stat sb;
-  int ret = stat(path.c_str(), &sb);
-  if (ret == -1 || !(sb.st_mode & S_IFREG))
-    return false;
-
-  return true;
-}
-
 bool UIMoveFile(const string& file, const string& newfile)
 {
   if (!rename(file.c_str(), newfile.c_str()))
@@ -893,30 +873,3 @@ bool UIMoveFile(const string& file, const string& newfile)
   [fileManager moveItemAtPath:source toPath:dest error:NULL];
   return UIFileExists(newfile);
 }
-
-bool UIDeleteFile(const string& file)
-{
-  return (unlink(file.c_str()) != -1);
-}
-
-std::ifstream* UIOpenRead(const string& filename)
-{
-  return new std::ifstream(filename.c_str(), std::ios::in);
-}
-
-std::ofstream* UIOpenWrite(const string& filename,
-                           bool append, // append=false
-                           bool binary) // binary=false
-{
-  std::ios_base::openmode mode = std::ios::out;
-
-  if (append) {
-    mode = mode | std::ios::app;
-  }
-
-  if (binary) {
-    mode = mode | std::ios::binary;
-  }
-
-  return new std::ofstream(filename.c_str(), mode);
-}

toolkit/crashreporter/client/crashreporter_unix_common.cpp

@@ -134,3 +134,56 @@ bool UIRunProgram(const std::string& exename, const std::string& arg,
 
   return true;
 }
+
+bool UIEnsurePathExists(const string& path)
+{
+  int ret = mkdir(path.c_str(), S_IRWXU);
+  int e = errno;
+  if (ret == -1 && e != EEXIST)
+    return false;
+
+  return true;
+}
+
+bool UIFileExists(const string& path)
+{
+  struct stat sb;
+  int ret = stat(path.c_str(), &sb);
+  if (ret == -1 || !(sb.st_mode & S_IFREG))
+    return false;
+
+  return true;
+}
+
+bool UIDeleteFile(const string& file)
+{
+  return (unlink(file.c_str()) != -1);
+}
+
+std::ifstream* UIOpenRead(const string& filename, bool binary)
+{
+  std::ios_base::openmode mode = std::ios::in;
+
+  if (binary) {
+    mode = mode | std::ios::binary;
+  }
+
+  return new std::ifstream(filename.c_str(), mode);
+}
+
+std::ofstream* UIOpenWrite(const string& filename,
+                           bool append, // append=false
+                           bool binary) // binary=false
+{
+  std::ios_base::openmode mode = std::ios::out;
+
+  if (append) {
+    mode = mode | std::ios::app;
+  }
+
+  if (binary) {
+    mode = mode | std::ios::binary;
+  }
+
+  return new std::ofstream(filename.c_str(), mode);
+}

toolkit/crashreporter/client/crashreporter_win.cpp

@@ -1463,16 +1463,21 @@ bool UIDeleteFile(const string& oldfile)
   return DeleteFile(UTF8ToWide(oldfile).c_str()) == TRUE;
 }
 
-ifstream* UIOpenRead(const string& filename)
+ifstream* UIOpenRead(const string& filename, bool binary)
 {
   // adapted from breakpad's src/common/windows/http_upload.cc
+  std::ios_base::openmode mode = ios::in;
+
+  if (binary) {
+    mode = mode | ios::binary;
+  }
 
 #if defined(_MSC_VER)
   ifstream* file = new ifstream();
-  file->open(UTF8ToWide(filename).c_str(), ios::in);
+  file->open(UTF8ToWide(filename).c_str(), mode);
 #else   // GCC
   ifstream* file = new ifstream(WideToMBCP(UTF8ToWide(filename), CP_ACP).c_str(),
-                                ios::in);
+                                mode);
 #endif  // _MSC_VER
 
   return file;

toolkit/crashreporter/client/moz.build

@@ -28,6 +28,7 @@ if CONFIG['OS_ARCH'] == 'WINNT':
     DEFINES['_UNICODE'] = True
     USE_LIBS += [
         'google_breakpad_libxul_s',
+        'nss',
     ]
     OS_LIBS += [
         'comctl32',
@@ -48,6 +49,7 @@ elif CONFIG['OS_ARCH'] == 'Darwin':
     USE_LIBS += [
         'breakpad_common_s',
         'breakpad_mac_common_s',
+        'nss',
     ]
 elif CONFIG['OS_ARCH'] == 'SunOS':
     SOURCES += [

toolkit/crashreporter/client/ping.cpp

@@ -164,7 +164,8 @@ CreateMetadataNode(StringTable& strings)
 
 // Create the payload node of the crash ping
 static Json::Value
-CreatePayloadNode(StringTable& strings, const string& aSessionId)
+CreatePayloadNode(StringTable& strings, const string& aHash,
+                  const string& aSessionId)
 {
   Json::Value payload;
 
@@ -174,6 +175,7 @@ CreatePayloadNode(StringTable& strings, const string& aSessionId)
   payload["crashTime"] = CurrentDate(kISO8601DateHours);
   payload["hasCrashEnvironment"] = true;
   payload["crashId"] = GetDumpLocalID();
+  payload["minidumpSha256Hash"] = aHash;
   payload["processType"] = "main"; // This is always a main crash
 
   // Parse the stack traces
@@ -219,7 +221,7 @@ CreateApplicationNode(const string& aVendor, const string& aName,
 
 // Create the root node of the crash ping
 static Json::Value
-CreateRootNode(StringTable& strings, const string& aUuid,
+CreateRootNode(StringTable& strings, const string& aUuid, const string& aHash,
                const string& aClientId, const string& aSessionId,
                const string& aName, const string& aVersion,
                const string& aChannel, const string& aBuildId)
@@ -252,7 +254,7 @@ CreateRootNode(StringTable& strings, const string& aUuid,
     root["environment"] = environment;
   }
 
-  root["payload"] = CreatePayloadNode(strings, aSessionId);
+  root["payload"] = CreatePayloadNode(strings, aHash, aSessionId);
   root["application"] = CreateApplicationNode(strings["Vendor"], aName,
                                               aVersion, aChannel, aBuildId,
                                               architecture, xpcomAbi);
@@ -282,7 +284,7 @@ GenerateSubmissionUrl(const string& aUrl, const string& aId,
 // Returns true if the ping was assembled and handed over to the pingsender
 // correctly, false otherwise and populates the aUUID field with the ping UUID.
 bool
-SendCrashPing(StringTable& strings, string& pingUuid)
+SendCrashPing(StringTable& strings, const string& aHash, string& pingUuid)
 {
   string clientId    = strings[kTelemetryClientId];
   string serverUrl   = strings[kTelemetryUrl];
@@ -305,7 +307,7 @@ SendCrashPing(StringTable& strings, string& pingUuid)
     return false;
   }
 
-  Json::Value root = CreateRootNode(strings, uuid, clientId, sessionId,
+  Json::Value root = CreateRootNode(strings, uuid, aHash, clientId, sessionId,
                                     name, version, channel, buildId);
 
   // Write out the result

toolkit/crashreporter/docs/index.rst

@@ -96,7 +96,8 @@ argument.
 The *crash reporter client* performs a number of roles. There's a lot going
 on, so you may want to look at ``main()`` in ``crashreporter.cpp``. First,
 stack traces are extracted from the dump via the *minidump analyzer* tool.
-The resulting traces are appended to the .extra file of the crash. Once this
+The resulting traces are appended to the .extra file of the crash together with
+the SHA256 hash of the minidump file. Once this
 is done a crash ping is assembled holding the same information as the one
 generated by the ```CrashManager``` and it's sent to the telemetry servers via
 the *ping sender* program. The UUID of the ping is then stored in the extra

toolkit/crashreporter/nsExceptionHandler.cpp

@@ -187,6 +187,9 @@ static XP_CHAR* crashReporterPath;
 static XP_CHAR* memoryReportPath;
 #if !defined(MOZ_WIDGET_ANDROID)
 static XP_CHAR* minidumpAnalyzerPath;
+#ifdef XP_MACOSX
+static XP_CHAR* libraryPath; // Path where the NSS library is
+#endif // XP_MACOSX
 #endif // !defined(MOZ_WIDGET_ANDROID)
 
 // Where crash events should go.
@@ -392,20 +395,6 @@ static const SIZE_T kReserveSize = 0x4000000; // 64 MB
 static void* gBreakpadReservedVM;
 #endif
 
-#ifdef XP_MACOSX
-static cpu_type_t pref_cpu_types[2] = {
-#if defined(__i386__)
-                                 CPU_TYPE_X86,
-#elif defined(__x86_64__)
-                                 CPU_TYPE_X86_64,
-#elif defined(__ppc__)
-                                 CPU_TYPE_POWERPC,
-#endif
-                                 CPU_TYPE_ANY };
-
-static posix_spawnattr_t spawnattr;
-#endif
-
 #if defined(MOZ_WIDGET_ANDROID)
 // Android builds use a custom library loader,
 // so the embedding will provide a list of shared
@@ -841,45 +830,27 @@ LaunchProgram(const XP_CHAR* aProgramPath, const XP_CHAR* aMinidumpPath,
     CloseHandle(pi.hThread);
   }
 #elif defined(XP_UNIX)
-#ifdef XP_MACOSX
-  pid_t pid = 0;
-  char* const my_argv[] = {
-    const_cast<char*>(aProgramPath),
-    const_cast<char*>(aMinidumpPath),
-    nullptr
-  };
-
-  char **env = nullptr;
-  char ***nsEnv = _NSGetEnviron();
-  if (nsEnv)
-    env = *nsEnv;
-
-  int rv = posix_spawnp(&pid, my_argv[0], nullptr, &spawnattr, my_argv, env);
-
-  if (rv != 0) {
-    return false;
-  } else if (aWait) {
-    waitpid(pid, nullptr, 0);
-  }
-
-#else // !XP_MACOSX
   pid_t pid = sys_fork();
 
   if (pid == -1) {
     return false;
   } else if (pid == 0) {
+#ifdef XP_LINUX
     // need to clobber this, as libcurl might load NSS,
     // and we want it to load the system NSS.
     unsetenv("LD_LIBRARY_PATH");
+#else // XP_MACOSX
+    // Needed to locate NSS and its dependencies
+    setenv("DYLD_LIBRARY_PATH", libraryPath, /* overwrite */ 1);
+#endif
     Unused << execl(aProgramPath,
                     aProgramPath, aMinidumpPath, (char*)0);
     _exit(1);
   } else {
     if (aWait) {
-      sys_waitpid(pid, nullptr, __WALL);
+      waitpid(pid, nullptr, 0);
     }
   }
-#endif // XP_MACOSX
 #endif // XP_UNIX
 
   return true;
@@ -1649,6 +1620,20 @@ nsresult SetExceptionHandler(nsIFile* aXREDirectory,
       return rv;
     }
 
+#ifdef XP_MACOSX
+    nsCOMPtr<nsIFile> libPath;
+    rv = aXREDirectory->Clone(getter_AddRefs(libPath));
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+        return rv;
+    }
+
+    nsAutoString libraryPath_temp;
+    rv = libPath->GetPath(libraryPath_temp);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+        return rv;
+    }
+#endif // XP_MACOSX
+
     nsAutoString minidumpAnalyzerPath_temp;
     rv = LocateExecutable(aXREDirectory,
                           NS_LITERAL_CSTRING(MINIDUMP_ANALYZER_FILENAME),
@@ -1665,6 +1650,9 @@ nsresult SetExceptionHandler(nsIFile* aXREDirectory,
 #else
   crashReporterPath = ToNewCString(crashReporterPath_temp);
   minidumpAnalyzerPath = ToNewCString(minidumpAnalyzerPath_temp);
+#ifdef XP_MACOSX
+  libraryPath = ToNewCString(libraryPath_temp);
+#endif
 #endif // XP_WIN32
 #else
     // On Android, we launch using the application package name instead of a
@@ -1692,25 +1680,6 @@ nsresult SetExceptionHandler(nsIFile* aXREDirectory,
     return NS_ERROR_FAILURE;
   }
 
-#ifdef XP_MACOSX
-  // Initialize spawn attributes, since this calls malloc.
-  if (posix_spawnattr_init(&spawnattr) != 0) {
-    return NS_ERROR_FAILURE;
-  }
-
-  // Set spawn attributes.
-  size_t attr_count = ArrayLength(pref_cpu_types);
-  size_t attr_ocount = 0;
-  if (posix_spawnattr_setbinpref_np(&spawnattr,
-                                    attr_count,
-                                    pref_cpu_types,
-                                    &attr_ocount) != 0 ||
-      attr_ocount != attr_count) {
-    posix_spawnattr_destroy(&spawnattr);
-    return NS_ERROR_FAILURE;
-  }
-#endif
-
 #ifdef XP_WIN32
   ReserveBreakpadVM();
 #endif // XP_WIN32
@@ -2141,6 +2110,19 @@ nsresult UnsetExceptionHandler()
     crashReporterPath = nullptr;
   }
 
+#if !defined(MOZ_WIDGET_ANDROID)
+  if (minidumpAnalyzerPath) {
+    free(minidumpAnalyzerPath);
+    minidumpAnalyzerPath = nullptr;
+  }
+#ifdef XP_MACOSX
+  if (libraryPath) {
+    free(libraryPath);
+    libraryPath = nullptr;
+  }
+#endif // XP_MACOSX
+#endif // !defined(MOZ_WIDGET_ANDROID)
+
   if (eventsDirectory) {
     free(eventsDirectory);
     eventsDirectory = nullptr;
@@ -2156,10 +2138,6 @@ nsresult UnsetExceptionHandler()
     memoryReportPath = nullptr;
   }
 
-#ifdef XP_MACOSX
-  posix_spawnattr_destroy(&spawnattr);
-#endif
-
   if (!gExceptionHandler)
     return NS_ERROR_NOT_INITIALIZED;
 
@@ -2955,9 +2933,7 @@ void
 DeleteMinidumpFilesForID(const nsAString& id)
 {
   nsCOMPtr<nsIFile> minidumpFile;
-  GetMinidumpForID(id, getter_AddRefs(minidumpFile));
+  if (GetMinidumpForID(id, getter_AddRefs(minidumpFile))) {
-  bool exists = false;
-  if (minidumpFile && NS_SUCCEEDED(minidumpFile->Exists(&exists)) && exists) {
     nsCOMPtr<nsIFile> childExtraFile;
     GetExtraFileForMinidump(minidumpFile, getter_AddRefs(childExtraFile));
     if (childExtraFile) {
@@ -2970,9 +2946,17 @@ DeleteMinidumpFilesForID(const nsAString& id)
 bool
 GetMinidumpForID(const nsAString& id, nsIFile** minidump)
 {
-  if (!GetMinidumpLimboDir(minidump))
+  if (!GetMinidumpLimboDir(minidump)) {
     return false;
+  }
+
   (*minidump)->Append(id + NS_LITERAL_STRING(".dmp"));
+
+  bool exists;
+  if (NS_FAILED((*minidump)->Exists(&exists)) || !exists) {
+    return false;
+  }
+
   return true;
 }
 
@@ -2989,9 +2973,17 @@ GetIDFromMinidump(nsIFile* minidump, nsAString& id)
 bool
 GetExtraFileForID(const nsAString& id, nsIFile** extraFile)
 {
-  if (!GetMinidumpLimboDir(extraFile))
+  if (!GetMinidumpLimboDir(extraFile)) {
     return false;
+  }
+
   (*extraFile)->Append(id + NS_LITERAL_STRING(".extra"));
+
+  bool exists;
+  if (NS_FAILED((*extraFile)->Exists(&exists)) || !exists) {
+    return false;
+  }
+
   return true;
 }
 
@@ -3042,7 +3034,7 @@ RunMinidumpAnalyzer(const nsAString& id)
 #if !defined(MOZ_WIDGET_ANDROID)
   nsCOMPtr<nsIFile> file;
 
-  if (CrashReporter::GetMinidumpForID(id, getter_AddRefs(file)) && file) {
+  if (CrashReporter::GetMinidumpForID(id, getter_AddRefs(file))) {
 #ifdef XP_WIN
     nsAutoString path;
     file->GetPath(path);

toolkit/xre/nsAppRunner.cpp

@@ -1239,6 +1239,26 @@ nsXULAppInfo::SetMinidumpPath(nsIFile* aMinidumpPath)
   return CrashReporter::SetMinidumpPath(path);
 }
 
+NS_IMETHODIMP
+nsXULAppInfo::GetMinidumpForID(const nsAString& aId, nsIFile** aMinidump)
+{
+  if (!CrashReporter::GetMinidumpForID(aId, aMinidump)) {
+    return NS_ERROR_FILE_NOT_FOUND;
+  }
+
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+nsXULAppInfo::GetExtraFileForID(const nsAString& aId, nsIFile** aExtraFile)
+{
+  if (!CrashReporter::GetExtraFileForID(aId, aExtraFile)) {
+    return NS_ERROR_FILE_NOT_FOUND;
+  }
+
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsXULAppInfo::AnnotateCrashReport(const nsACString& key,
                                   const nsACString& data)

xpcom/system/nsICrashReporter.idl

@@ -48,6 +48,30 @@ interface nsICrashReporter : nsISupports
    */
   attribute nsIFile minidumpPath;
 
+  /**
+   * Get the minidump file corresponding to the specified ID.
+   *
+   * @param id
+   *        ID of the crash. Likely a UUID.
+   *
+   * @return The minidump file associated with the ID.
+   *
+   * @throw NS_ERROR_FILE_NOT_FOUND if the minidump could not be found
+   */
+  nsIFile getMinidumpForID(in AString id);
+
+  /**
+   * Get the extra file corresponding to the specified ID.
+   *
+   * @param id
+   *        ID of the crash. Likely a UUID.
+   *
+   * @return The extra file associated with the ID.
+   *
+   * @throw NS_ERROR_FILE_NOT_FOUND if the extra file could not be found
+   */
+  nsIFile getExtraFileForID(in AString id);
+
   /**
    * Add some extra data to be submitted with a crash report.
    *