Implement anonymous, opt-in, collection of OS X binary integrity incidents.

chrome/browser/safe_browsing/signature_evaluator_mac_unittest.cc

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/safe_browsing/signature_evaluator_mac.h"
+
+#include <CoreFoundation/CoreFoundation.h>
+#include <sys/xattr.h>
+
+#include <string>
+#include <vector>
+
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/mac/mac_util.h"
+#include "base/mac/scoped_cftyperef.h"
+#include "base/path_service.h"
+#include "base/test/scoped_path_override.h"
+#include "chrome/browser/safe_browsing/incident_reporting/incident.h"
+#include "chrome/browser/safe_browsing/incident_reporting/mock_incident_receiver.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/common/safe_browsing/csd.pb.h"
+#include "testing/gmock/include/gmock/gmock-matchers.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ::testing::_;
+using ::testing::StrictMock;
+
+namespace safe_browsing {
+
+namespace {
+
+const char* const xattrs[] = {
+  "com.apple.cs.CodeDirectory",
+  "com.apple.cs.CodeSignature",
+  "com.apple.cs.CodeRequirements",
+  "com.apple.cs.CodeResources",
+  "com.apple.cs.CodeApplication",
+  "com.apple.cs.CodeEntitlements",
+};
+
+}  // namespace

[Robert Sesek, 2015/10/08 19:20:06]

nit: remove? there's a closing namespace online 329

[Greg K, 2015/10/09 17:12:01]

The one at the bottom was incorrectly commented. I fixed it to point out that it
closes the safe_browsing namespace.

+
+class MacSignatureEvaluatorTest : public testing::Test {
+ protected:
+  void SetUp() override {
+    base::FilePath source_path;
+    ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &source_path));
+    testdata_path_ =
+        source_path.AppendASCII("safe_browsing").AppendASCII("mach_o");
+
+    base::FilePath dir_exe;
+    ASSERT_TRUE(PathService::Get(base::DIR_EXE, &dir_exe));
+    base::FilePath file_exe;
+    ASSERT_TRUE(PathService::Get(base::FILE_EXE, &file_exe));
+
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+  }
+
+  bool GetExecPath(const base::FilePath& bundle_url, base::FilePath* result) {
+    base::ScopedCFTypeRef<CFStringRef> path_str(CFStringCreateWithCString(

[Robert Sesek, 2015/10/08 19:20:06]

SysUTF8ToCFStringRef

[Greg K, 2015/10/09 17:12:01]

Done.

+        kCFAllocatorDefault, bundle_url.value().c_str(),
+        kCFStringEncodingUTF8));
+    if (!path_str.get())
+      return false;
+    base::ScopedCFTypeRef<CFURLRef> path_url(CFURLCreateWithFileSystemPath(
+        kCFAllocatorDefault, path_str, kCFURLPOSIXPathStyle, false));
+    if (!path_url.get())
+      return false;
+    base::ScopedCFTypeRef<CFBundleRef> bundle(
+        CFBundleCreate(kCFAllocatorDefault, path_url));
+    if (!bundle.get())
+      return false;
+
+    base::ScopedCFTypeRef<CFURLRef> exec_url(CFBundleCopyExecutableURL(bundle));
+    UInt8 path_buf[PATH_MAX];
+    if (!CFURLGetFileSystemRepresentation(exec_url, true, path_buf,
+                                          sizeof(path_buf)))

[Robert Sesek, 2015/10/08 19:20:06]

nit: needs braces

[Greg K, 2015/10/09 17:12:01]

Done.

+      return false;
+
+    *result = base::FilePath(reinterpret_cast<const char*>(path_buf));
+    return true;
+  }
+
+  bool SetupXattrs(const base::FilePath& path) {
+    char sentinel = 'A';
+    for (const auto& xattr : xattrs) {
+      std::vector<uint8_t> buf(10);
+      memset(&buf[0], sentinel++, buf.size());
+      if (setxattr(path.value().c_str(), xattr, &buf[0], buf.size(), 0, 0) != 0)
+        return false;
+    }
+    return true;
+  }
+
+  base::FilePath testdata_path_;
+  base::ScopedTempDir temp_dir_;
+};
+
+TEST_F(MacSignatureEvaluatorTest, SimpleTest) {
+  // This is a simple test that checks the validity of a signed executable.
+  // There is no designated requirement: we only check the embedded signature.
+  base::FilePath path = testdata_path_.AppendASCII("signedexecutablefat");
+  safe_browsing::MacSignatureEvaluator evaluator(path);
+  ASSERT_TRUE(evaluator.Initialize());
+
+  std::vector<ClientIncidentReport_IncidentData_BinaryIntegrityIncident>
+      results;
+  ASSERT_TRUE(evaluator.PerformEvaluation(results));

[Robert Sesek, 2015/10/08 19:20:06]

EXPECT  (and throughout where possible)

[Greg K, 2015/10/09 17:12:01]

Done.

+  ASSERT_EQ(0u, results.size());
+}
+
+TEST_F(MacSignatureEvaluatorTest, SimpleTestWithDR) {
+  // This test checks the signer against a designated requirement description.
+  base::FilePath path = testdata_path_.AppendASCII("signedexecutablefat");
+  std::string requirement(
+      "certificate leaf[subject.CN]=\"untrusted@goat.local\"");
+  safe_browsing::MacSignatureEvaluator evaluator(path, requirement);
+  ASSERT_TRUE(evaluator.Initialize());
+
+  std::vector<ClientIncidentReport_IncidentData_BinaryIntegrityIncident>
+      results;
+  ASSERT_TRUE(evaluator.PerformEvaluation(results));
+  ASSERT_EQ(0u, results.size());
+}
+
+TEST_F(MacSignatureEvaluatorTest, SimpleTestWithBadDR) {
+  // Now test with a designated requirement that does not describe the signer.
+  base::FilePath path = testdata_path_.AppendASCII("signedexecutablefat");
+  safe_browsing::MacSignatureEvaluator evaluator(path, "anchor apple");
+  ASSERT_TRUE(evaluator.Initialize());
+
+  std::vector<ClientIncidentReport_IncidentData_BinaryIntegrityIncident>
+      results;
+  ASSERT_FALSE(evaluator.PerformEvaluation(results));
+  ASSERT_EQ(1u, results.size());

[Robert Sesek, 2015/10/08 19:20:06]

This has to remain an ASSERT_ since you get results[0] below, but the others can
be EXPECT_.

[Greg K, 2015/10/09 17:12:01]

Acknowledged.

+
+  const ClientIncidentReport_IncidentData_BinaryIntegrityIncident& result =
+      results[0];
+  ASSERT_EQ(-67050, result.sec_error());
+  ASSERT_TRUE(result.has_file_basename());
+  ASSERT_EQ("signedexecutablefat", result.file_basename());
+  ASSERT_TRUE(result.has_signature());
+}
+
+TEST_F(MacSignatureEvaluatorTest, SimpleBundleTest) {
+  // Now test a simple, validly signed bundle.
+  base::FilePath path = testdata_path_.AppendASCII("test-bundle.app");
+  base::FilePath exec_path;
+  ASSERT_TRUE(GetExecPath(path, &exec_path));
+
+  std::string requirement(
+      "certificate leaf[subject.CN]=\"untrusted@goat.local\"");
+  safe_browsing::MacSignatureEvaluator evaluator(exec_path, requirement);
+  ASSERT_TRUE(evaluator.Initialize());
+
+  std::vector<ClientIncidentReport_IncidentData_BinaryIntegrityIncident>
+      results;
+  ASSERT_TRUE(evaluator.PerformEvaluation(results));
+  ASSERT_EQ(0u, results.size());
+}
+
+TEST_F(MacSignatureEvaluatorTest, ModifiedMainExecTest32) {
+  // Now to a test modified, signed bundle.
+  base::FilePath path = testdata_path_.AppendASCII("modified-main-exec32.app");
+  base::FilePath exec_path;
+  ASSERT_TRUE(GetExecPath(path, &exec_path));
+
+  std::string requirement(
+      "certificate leaf[subject.CN]=\"untrusted@goat.local\"");
+  safe_browsing::MacSignatureEvaluator evaluator(exec_path, requirement);
+  ASSERT_TRUE(evaluator.Initialize());
+
+  std::vector<ClientIncidentReport_IncidentData_BinaryIntegrityIncident>
+      results;
+  ASSERT_FALSE(evaluator.PerformEvaluation(results));
+  ASSERT_EQ(1u, results.size());
+
+  const ClientIncidentReport_IncidentData_BinaryIntegrityIncident& incident =
+      results[0];
+  ASSERT_EQ(-67061, incident.sec_error());
+  ASSERT_EQ(exec_path.BaseName().value(), incident.file_basename());
+}
+
+TEST_F(MacSignatureEvaluatorTest, ModifiedMainExecTest64) {
+  // Snow Leopard does not know about the 64-bit slice so this test is
+  // irrelevant.
+  if (base::mac::IsOSLionOrLater()) {

[Robert Sesek, 2015/10/08 19:20:06]

Generally prefer early returns for short-circuiting.

[Greg K, 2015/10/09 17:12:01]

Done.

+    // Now to a test modified, signed bundle.
+    base::FilePath path =
+        testdata_path_.AppendASCII("modified-main-exec64.app");
+    base::FilePath exec_path;
+    ASSERT_TRUE(GetExecPath(path, &exec_path));
+
+    std::string requirement(
+        "certificate leaf[subject.CN]=\"untrusted@goat.local\"");
+    safe_browsing::MacSignatureEvaluator evaluator(exec_path, requirement);
+    ASSERT_TRUE(evaluator.Initialize());
+
+    std::vector<ClientIncidentReport_IncidentData_BinaryIntegrityIncident>
+        results;
+    ASSERT_FALSE(evaluator.PerformEvaluation(results));
+    ASSERT_EQ(1u, results.size());
+
+    const ClientIncidentReport_IncidentData_BinaryIntegrityIncident& incident =
+        results[0];
+    ASSERT_EQ(-67061, incident.sec_error());
+    ASSERT_EQ(exec_path.BaseName().value(), incident.file_basename());
+  }
+}
+
+TEST_F(MacSignatureEvaluatorTest, ModifiedBundleAndExecTest) {
+  // Now test a modified, signed bundle with resources added and the main
+  // executable modified.
+  base::FilePath path =
+      testdata_path_.AppendASCII("modified-bundle-and-exec.app");
+  base::FilePath exec_path;
+  ASSERT_TRUE(GetExecPath(path, &exec_path));
+
+  std::string requirement(
+      "certificate leaf[subject.CN]=\"untrusted@goat.local\"");
+  safe_browsing::MacSignatureEvaluator evaluator(exec_path, requirement);
+  ASSERT_TRUE(evaluator.Initialize());
+
+  std::vector<ClientIncidentReport_IncidentData_BinaryIntegrityIncident>
+      results;
+  ASSERT_FALSE(evaluator.PerformEvaluation(results));
+  ASSERT_EQ(1u, results.size());
+
+  const ClientIncidentReport_IncidentData_BinaryIntegrityIncident& incident1 =
+      results[0];
+  ASSERT_TRUE(incident1.has_file_basename());
+  ASSERT_EQ(exec_path.BaseName().value(), incident1.file_basename());
+  ASSERT_EQ(-67061, incident1.sec_error());
+  ASSERT_TRUE(incident1.has_signature());
+}
+
+TEST_F(MacSignatureEvaluatorTest, ModifiedBundleTest) {
+  // Now test a modified, signed bundle. This bundle has
+  // the following problems:
+  // 1) A file was added (This should not be reported)
+  // 2) libsigned64.dylib was modified
+  // 3) executable32 was modified
+
+  base::FilePath orig_path = testdata_path_.AppendASCII("modified-bundle.app");
+  base::FilePath copied_path =
+      temp_dir_.path().AppendASCII("modified-bundle.app");
+  CHECK(base::CopyDirectory(orig_path, copied_path, true));
+
+  base::FilePath exec_path;
+  ASSERT_TRUE(GetExecPath(copied_path, &exec_path));
+
+  // Setup the extended attributes, which don't persist in the git repo.
+  ASSERT_TRUE(SetupXattrs(
+      copied_path.AppendASCII("Contents/Resources/Base.lproj/MainMenu.nib")));
+
+  std::string requirement(
+      "certificate leaf[subject.CN]=\"untrusted@goat.local\"");
+  safe_browsing::MacSignatureEvaluator evaluator(exec_path, requirement);
+  ASSERT_TRUE(evaluator.Initialize());
+
+  std::vector<ClientIncidentReport_IncidentData_BinaryIntegrityIncident>
+      results;
+  ASSERT_FALSE(evaluator.PerformEvaluation(results));
+  ASSERT_EQ(4u, results.size());
+
+  const ClientIncidentReport_IncidentData_BinaryIntegrityIncident* main_exec =
+      nullptr;
+  const ClientIncidentReport_IncidentData_BinaryIntegrityIncident* libsigned64 =
+      nullptr;
+  const ClientIncidentReport_IncidentData_BinaryIntegrityIncident*
+      executable32 = nullptr;
+  const ClientIncidentReport_IncidentData_BinaryIntegrityIncident* mainmenunib =
+      nullptr;
+  const ClientIncidentReport_IncidentData_BinaryIntegrityIncident*
+      codesign_cfg = nullptr;
+
+  for (const auto& incident : results) {
+    if (incident.file_basename() == exec_path.BaseName().value())
+      main_exec = &incident;
+    else if (incident.file_basename() == "libsigned64.dylib")
+      libsigned64 = &incident;
+    else if (incident.file_basename() == "executable32")
+      executable32 = &incident;
+    else if (incident.file_basename() == "MainMenu.nib")
+      mainmenunib = &incident;
+    else if (incident.file_basename() == "codesign.cfg")
+      codesign_cfg = &incident;
+  }
+  ASSERT_NE(main_exec, nullptr);

[Robert Sesek, 2015/10/08 19:20:06]

Similarly, these should be ASSERT_ since they're dereferenced below, but then
you can use EXPECT_ for verifying the data.

[Greg K, 2015/10/09 17:12:01]

Done.

+  ASSERT_NE(libsigned64, nullptr);
+  ASSERT_NE(executable32, nullptr);
+  // This is important. Do not collect information on extra files added.
+  ASSERT_EQ(codesign_cfg, nullptr);
+
+  ASSERT_TRUE(main_exec->has_file_basename());
+  ASSERT_EQ(exec_path.BaseName().value(), main_exec->file_basename());
+  ASSERT_TRUE(main_exec->has_signature());
+  ASSERT_EQ(-67054, main_exec->sec_error());
+
+  ASSERT_TRUE(libsigned64->has_file_basename());
+  ASSERT_EQ("libsigned64.dylib", libsigned64->file_basename());
+  ASSERT_TRUE(libsigned64->has_signature());
+
+  ASSERT_TRUE(executable32->has_file_basename());
+  ASSERT_EQ("executable32", executable32->file_basename());
+  ASSERT_TRUE(executable32->has_signature());
+
+  ASSERT_TRUE(mainmenunib->has_file_basename());
+  ASSERT_EQ("MainMenu.nib", mainmenunib->file_basename());
+  ASSERT_TRUE(mainmenunib->has_signature());
+  ASSERT_EQ(6, mainmenunib->signature().xattr_size());
+  // Manually convert the global xattrs array to a vector
+  std::vector<std::string> xattrs_known;
+  for (const auto& xattr : xattrs)
+    xattrs_known.push_back(xattr);
+
+  std::vector<std::string> xattrs_seen;
+  for (const auto& xattr : mainmenunib->signature().xattr()) {
+    ASSERT_TRUE(xattr.has_key());
+    ASSERT_TRUE(xattr.has_value());
+    xattrs_seen.push_back(xattr.key());
+  }
+  ASSERT_THAT(xattrs_known, ::testing::ContainerEq(xattrs_seen));
+}
+
+}  // namespace