crash_reporter: Fix product_id in kernel crash reports

Move the bdk_version, product_id, and product_version metadata
population into the base crash_collector class so that all
colletors correctly report those fields in crash reports.

BUG=27344416
TEST=crash_reporter_tests passes.

Change-Id: I050053055f197d01661a1442e3cdcccc53c1c8fe

4 files changed, 69 insertions, 30 deletions

diff --git a/crash_reporter/crash_collector.cc b/crash_reporter/crash_collector.cc
index b3fdcb4a8..31d9f0f6d 100644
--- a/crash_reporter/crash_collector.cc
+++ b/crash_reporter/crash_collector.cc
@@ -36,6 +36,7 @@
 #include <base/strings/string_util.h>
 #include <base/strings/stringprintf.h>
 #include <brillo/key_value_store.h>
+#include <brillo/osrelease_reader.h>
 #include <brillo/process.h>
 
 namespace {
@@ -52,6 +53,11 @@ const char kSystemCrashPath[] = "/data/misc/crash_reporter/crash";
 const char kUploadVarPrefix[] = "upload_var_";
 const char kUploadFilePrefix[] = "upload_file_";
 
+// Product information keys in the /etc/os-release.d folder.
+static const char kBdkVersionKey[] = "bdk_version";
+static const char kProductIDKey[] = "product_id";
+static const char kProductVersionKey[] = "product_version";
+
 // Normally this path is not used.  Unfortunately, there are a few edge cases
 // where we need this.  Any process that runs as kDefaultUserName that crashes
 // is consider a "user crash".  That includes the initial Chrome browser that
@@ -384,14 +390,49 @@ void CrashCollector::WriteCrashMetaData(const FilePath &meta_path,
                                         const std::string &payload_path) {
   int64_t payload_size = -1;
   base::GetFileSize(FilePath(payload_path), &payload_size);
+
+  brillo::OsReleaseReader reader;
+  if (!forced_osreleased_directory_.empty()) {
+    reader.LoadTestingOnly(forced_osreleased_directory_);
+  } else {
+    reader.Load();
+  }
+  std::string bdk_version = "undefined";
+  std::string product_id = "undefined";
+  std::string product_version = "undefined";
+
+  if (!reader.GetString(kBdkVersionKey, &bdk_version)) {
+    LOG(ERROR) << "Could not read " << kBdkVersionKey
+               << " from /etc/os-release.d/";
+  }
+
+  if (!reader.GetString(kProductIDKey, &product_id)) {
+    LOG(ERROR) << "Could not read " << kProductIDKey
+               << " from /etc/os-release.d/";
+  }
+
+  if (!reader.GetString(kProductVersionKey, &product_version)) {
+    LOG(ERROR) << "Could not read " << kProductVersionKey
+               << " from /etc/os-release.d/";
+  }
+
   std::string meta_data = StringPrintf("%sexec_name=%s\n"
                                        "payload=%s\n"
                                        "payload_size=%" PRId64 "\n"
+                                       "%s=%s\n"
+                                       "%s=%s\n"
+                                       "%s=%s\n"
                                        "done=1\n",
                                        extra_metadata_.c_str(),
                                        exec_name.c_str(),
                                        payload_path.c_str(),
-                                       payload_size);
+                                       payload_size,
+                                       kBdkVersionKey,
+                                       bdk_version.c_str(),
+                                       kProductIDKey,
+                                       product_id.c_str(),
+                                       kProductVersionKey,
+                                       product_version.c_str());
   // We must use WriteNewFile instead of base::WriteFile as we
   // do not want to write with root access to a symlink that an attacker
   // might have created.
diff --git a/crash_reporter/crash_collector.h b/crash_reporter/crash_collector.h
index 24cbfb311..21b919872 100644
--- a/crash_reporter/crash_collector.h
+++ b/crash_reporter/crash_collector.h
@@ -84,6 +84,12 @@ class CrashCollector {
     forced_crash_directory_ = forced_directory;
   }
 
+  // For testing, set the root directory to read etc/os-release.d properties
+  // from.
+  void ForceOsReleaseDDirectory(const base::FilePath &forced_directory) {
+    forced_osreleased_directory_ = forced_directory;
+  }
+
   base::FilePath GetCrashDirectoryInfo(mode_t *mode,
                                        uid_t *directory_owner,
                                        gid_t *directory_group);
@@ -158,6 +164,7 @@ class CrashCollector {
   IsFeedbackAllowedFunction is_feedback_allowed_function_;
   std::string extra_metadata_;
   base::FilePath forced_crash_directory_;
+  base::FilePath forced_osreleased_directory_;
   base::FilePath log_config_path_;
 
  private:
diff --git a/crash_reporter/crash_collector_test.cc b/crash_reporter/crash_collector_test.cc
index 11c8c0d82..90a209d08 100644
--- a/crash_reporter/crash_collector_test.cc
+++ b/crash_reporter/crash_collector_test.cc
@@ -182,9 +182,26 @@ TEST_F(CrashCollectorTest, MetaData) {
   const char kMetaFileBasename[] = "generated.meta";
   FilePath meta_file = test_dir_.path().Append(kMetaFileBasename);
   FilePath payload_file = test_dir_.path().Append("payload-file");
+  FilePath osreleased_directory =
+      test_dir_.path().Append("etc").Append("os-release.d");
+  ASSERT_TRUE(base::CreateDirectory(osreleased_directory));
+  collector_.ForceOsReleaseDDirectory(test_dir_.path());
+
   std::string contents;
   const char kPayload[] = "foo";
   ASSERT_TRUE(base::WriteFile(payload_file, kPayload, strlen(kPayload)));
+  const char kBdkVersion[] = "1";
+  ASSERT_TRUE(base::WriteFile(osreleased_directory.Append("bdk_version"),
+                              kBdkVersion,
+                              strlen(kBdkVersion)));
+  const char kProductId[] = "baz";
+  ASSERT_TRUE(base::WriteFile(osreleased_directory.Append("product_id"),
+                              kProductId,
+                              strlen(kProductId)));
+  const char kProductVersion[] = "1.2.3.4";
+  ASSERT_TRUE(base::WriteFile(osreleased_directory.Append("product_version"),
+                              kProductVersion,
+                              strlen(kProductVersion)));
   collector_.AddCrashMetaData("foo", "bar");
   collector_.WriteCrashMetaData(meta_file, "kernel", payload_file.value());
   EXPECT_TRUE(base::ReadFileToString(meta_file, &contents));
@@ -193,6 +210,9 @@ TEST_F(CrashCollectorTest, MetaData) {
           "exec_name=kernel\n"
           "payload=%s\n"
           "payload_size=3\n"
+          "bdk_version=1\n"
+          "product_id=baz\n"
+          "product_version=1.2.3.4\n"
           "done=1\n",
           test_dir_.path().Append("payload-file").value().c_str());
   EXPECT_EQ(kExpectedMeta, contents);
diff --git a/crash_reporter/user_collector.cc b/crash_reporter/user_collector.cc
index 98d7448f7..48b64e917 100644
--- a/crash_reporter/user_collector.cc
+++ b/crash_reporter/user_collector.cc
@@ -37,7 +37,6 @@
 #include <base/strings/string_split.h>
 #include <base/strings/string_util.h>
 #include <base/strings/stringprintf.h>
-#include <brillo/osrelease_reader.h>
 #include <brillo/process.h>
 #include <brillo/syslog_logging.h>
 #include <cutils/properties.h>
@@ -59,11 +58,6 @@ static const gid_t kUnknownGid = -1;
 const char *UserCollector::kUserId = "Uid:\t";
 const char *UserCollector::kGroupId = "Gid:\t";
 
-// Product information keys in the /etc/os-release.d folder.
-static const char kBdkVersionKey[] = "bdk_version";
-static const char kProductIDKey[] = "product_id";
-static const char kProductVersionKey[] = "product_version";
-
 
 using base::FilePath;
 using base::StringPrintf;
@@ -505,29 +499,6 @@ UserCollector::ErrorType UserCollector::ConvertAndEnqueueCrash(
   if (GetLogContents(FilePath(log_config_path_), exec, log_path))
     AddCrashMetaData("log", log_path.value());
 
-  brillo::OsReleaseReader reader;
-  reader.Load();
-  std::string value = "undefined";
-  if (!reader.GetString(kBdkVersionKey, &value)) {
-    LOG(ERROR) << "Could not read " << kBdkVersionKey
-               << " from /etc/os-release.d/";
-  }
-  AddCrashMetaData(kBdkVersionKey, value);
-
-  value = "undefined";
-  if (!reader.GetString(kProductIDKey, &value)) {
-    LOG(ERROR) << "Could not read " << kProductIDKey
-               << " from /etc/os-release.d/";
-  }
-  AddCrashMetaData(kProductIDKey, value);
-
-  value = "undefined";
-  if (!reader.GetString(kProductVersionKey, &value)) {
-    LOG(ERROR) << "Could not read " << kProductVersionKey
-               << " from /etc/os-release.d/";
-  }
-  AddCrashMetaData(kProductVersionKey, value);
-
   ErrorType error_type =
       ConvertCoreToMinidump(pid, container_dir, core_path, minidump_path);
   if (error_type != kErrorNone) {