Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(208)

Unified Diff: chrome/browser/safe_browsing/signature_evaluator_mac_unittest.cc

Issue 1363613004: Implement anonymous, opt-in, collection of OS X binary integrity incidents. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/safe_browsing/signature_evaluator_mac_unittest.cc
diff --git a/chrome/browser/safe_browsing/signature_evaluator_mac_unittest.cc b/chrome/browser/safe_browsing/signature_evaluator_mac_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..e1e5c1b72319da300fb41c1a0acc92baa70d2e24
--- /dev/null
+++ b/chrome/browser/safe_browsing/signature_evaluator_mac_unittest.cc
@@ -0,0 +1,328 @@
+// 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 <string>
Mark Mentovai 2015/10/05 15:02:12 Separate C from C++ system headers.
Greg K 2015/10/07 22:54:30 Done.
+#include <sys/xattr.h>
+#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/common/chrome_paths.h"
+#include "chrome/common/safe_browsing/csd.pb.h"
+#include "testing/gmock/include/gmock/gmock-matchers.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+const char* xattrs[] = {
Mark Mentovai 2015/10/05 15:02:12 It’s unfortunate that this isn’t sharing with the
Greg K 2015/10/07 22:54:30 I actually made these two separate lists on purpos
+ "com.apple.cs.CodeDirectory", "com.apple.cs.CodeSignature",
+ "com.apple.cs.CodeRequirements", "com.apple.cs.CodeResources",
+ "com.apple.cs.CodeApplication", "com.apple.cs.CodeEntitlements",
+};
+}
Mark Mentovai 2015/10/05 15:02:12 // namespace
Greg K 2015/10/07 22:54:30 Done.
+
+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));
+
+ CHECK(temp_dir_.CreateUniqueTempDir());
Robert Sesek 2015/10/05 22:19:07 No CHECK
Greg K 2015/10/07 22:54:30 Done.
+ }
+
+ bool GetExecPath(const base::FilePath& bundle_url, base::FilePath* result) {
+ base::ScopedCFTypeRef<CFStringRef> path_str(CFStringCreateWithCString(
+ 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)))
+ 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());
+
+ safe_browsing::ClientIncidentReport_IncidentData_OSXBinaryIntegrityIncident
+ result;
+ ASSERT_TRUE(evaluator.PerformEvaluation(&result));
+ ASSERT_EQ(0, result.sub_incident_size());
+ ASSERT_FALSE(result.has_sec_error());
+ ASSERT_FALSE(result.has_file_basename());
+}
+
+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());
+
+ safe_browsing::ClientIncidentReport_IncidentData_OSXBinaryIntegrityIncident
+ result;
+ ASSERT_TRUE(evaluator.PerformEvaluation(&result));
+ ASSERT_EQ(0, result.sub_incident_size());
+ ASSERT_FALSE(result.has_sec_error());
+ ASSERT_FALSE(result.has_file_basename());
+}
+
+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());
+
+ safe_browsing::ClientIncidentReport_IncidentData_OSXBinaryIntegrityIncident
+ result;
+ ASSERT_FALSE(evaluator.PerformEvaluation(&result));
+ ASSERT_EQ(1, result.sub_incident_size());
+ ASSERT_EQ(-67050, result.sec_error());
+
+ const safe_browsing::
+ ClientIncidentReport_IncidentData_BinaryIntegrityIncident& incident =
+ result.sub_incident(0);
+ ASSERT_TRUE(incident.has_file_basename());
+ ASSERT_EQ("signedexecutablefat", incident.file_basename());
+ ASSERT_TRUE(incident.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());
+
+ safe_browsing::ClientIncidentReport_IncidentData_OSXBinaryIntegrityIncident
+ result;
+ ASSERT_TRUE(evaluator.PerformEvaluation(&result));
+ ASSERT_EQ(0, result.sub_incident_size());
+ ASSERT_FALSE(result.has_sec_error());
+ ASSERT_FALSE(result.has_file_basename());
+}
+
+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());
+
+ safe_browsing::ClientIncidentReport_IncidentData_OSXBinaryIntegrityIncident
+ incident;
+ ASSERT_FALSE(evaluator.PerformEvaluation(&incident));
+ ASSERT_EQ(1, incident.sub_incident_size());
+ 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()) {
+ // 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());
+
+ safe_browsing::ClientIncidentReport_IncidentData_OSXBinaryIntegrityIncident
+ incident;
+ ASSERT_FALSE(evaluator.PerformEvaluation(&incident));
+ ASSERT_EQ(1, incident.sub_incident_size());
+ 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());
+
+ safe_browsing::ClientIncidentReport_IncidentData_OSXBinaryIntegrityIncident
+ result;
+ ASSERT_FALSE(evaluator.PerformEvaluation(&result));
+ ASSERT_EQ(-67061, result.sec_error());
+
+ ASSERT_EQ(exec_path.BaseName().value(), result.file_basename());
+ ASSERT_EQ(1, result.sub_incident_size());
+
+ const safe_browsing::
+ ClientIncidentReport_IncidentData_BinaryIntegrityIncident& sub_incident =
+ result.sub_incident(0);
+ ASSERT_TRUE(sub_incident.has_file_basename());
+ ASSERT_EQ(sub_incident.file_basename(), exec_path.BaseName().value());
+ ASSERT_TRUE(sub_incident.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());
+
+ safe_browsing::ClientIncidentReport_IncidentData_OSXBinaryIntegrityIncident
+ result;
+ ASSERT_FALSE(evaluator.PerformEvaluation(&result));
+ ASSERT_EQ(-67054, result.sec_error());
+ ASSERT_EQ(exec_path.BaseName().value(), result.file_basename());
+ ASSERT_EQ(4, result.sub_incident_size());
+
+ const google::protobuf::RepeatedPtrField<
+ safe_browsing::ClientIncidentReport_IncidentData_BinaryIntegrityIncident>&
+ incidents = result.sub_incident();
+ const safe_browsing::
+ ClientIncidentReport_IncidentData_BinaryIntegrityIncident* main_exec =
+ nullptr;
+ const safe_browsing::
+ ClientIncidentReport_IncidentData_BinaryIntegrityIncident* libsigned64 =
+ nullptr;
+ const safe_browsing::
+ ClientIncidentReport_IncidentData_BinaryIntegrityIncident* executable32 =
+ nullptr;
+ const safe_browsing::
+ ClientIncidentReport_IncidentData_BinaryIntegrityIncident* mainmenunib =
+ nullptr;
+ const safe_browsing::
+ ClientIncidentReport_IncidentData_BinaryIntegrityIncident* codesign_cfg =
+ nullptr;
+
+ for (const auto& incident : incidents) {
+ 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);
+ 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_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));
+}

Powered by Google App Engine
This is Rietveld 408576698