diff options
author | Sen Jiang <senj@google.com> | 2018-08-14 14:02:32 -0700 |
---|---|---|
committer | android-build-merger <android-build-merger@google.com> | 2018-08-14 14:02:32 -0700 |
commit | 02903ef05eeace7afcd50207b7bd04a6ec7391e0 (patch) | |
tree | 279e9e692e3a7db704f65e6d3977e43e8e75b46a | |
parent | 5d78951d5bc7bad54082b1083c2dc7f3b9fae89d (diff) | |
parent | 0fa01c38cc2beff8d7e11e4b512692ecdb640ca8 (diff) | |
download | platform_external_libbrillo-02903ef05eeace7afcd50207b7bd04a6ec7391e0.tar.gz platform_external_libbrillo-02903ef05eeace7afcd50207b7bd04a6ec7391e0.tar.bz2 platform_external_libbrillo-02903ef05eeace7afcd50207b7bd04a6ec7391e0.zip |
Merge remote-tracking branch 'aosp/upstream-master' into aosp/master.
am: 0fa01c38cc
Change-Id: Ifc725167753e0e2c9d5a7f966c70fa22329166e3
89 files changed, 3256 insertions, 1033 deletions
@@ -22,6 +22,7 @@ libbrillo_core_sources = [ "brillo/errors/error.cc", "brillo/errors/error_codes.cc", "brillo/flag_helper.cc", + "brillo/imageloader/manifest.cc", "brillo/key_value_store.cc", "brillo/message_loops/base_message_loop.cc", "brillo/message_loops/message_loop.cc", @@ -97,6 +98,7 @@ libbrillo_test_sources = [ "brillo/http/http_request_unittest.cc", "brillo/http/http_transport_curl_unittest.cc", "brillo/http/http_utils_unittest.cc", + "brillo/imageloader/manifest_unittest.cc", "brillo/key_value_store_unittest.cc", "brillo/map_utils_unittest.cc", "brillo/message_loops/base_message_loop_unittest.cc", @@ -292,4 +294,3 @@ cc_test { cflags: libbrillo_CFLAGS, cppflags: ["-Wno-sign-compare"], } - @@ -10,3 +10,4 @@ stevefung@google.com benchan@google.com derat@google.com vapier@google.com +ejcaruso@google.com diff --git a/brillo/asan.h b/brillo/asan.h new file mode 100644 index 0000000..9a73202 --- /dev/null +++ b/brillo/asan.h @@ -0,0 +1,21 @@ +// Copyright 2018 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +// +// This header provides Address Sanitizer specific macros. +// +#ifndef LIBBRILLO_BRILLO_ASAN_H_ +#define LIBBRILLO_BRILLO_ASAN_H_ + +#if defined(__has_feature) && __has_feature(address_sanitizer) +// ASan is enabled. +#define BRILLO_ASAN_BUILD 1 +// Provide BRILLO_DISABLE_ASAN hook to disable ASan. +// Put this in front on functions or global variables where required. +#define BRILLO_DISABLE_ASAN __attribute__((no_sanitize("address"))) +#else +#define BRILLO_DISABLE_ASAN +#endif + +#endif + diff --git a/brillo/cryptohome.cc b/brillo/cryptohome.cc index 49a4a88..88e4739 100644 --- a/brillo/cryptohome.cc +++ b/brillo/cryptohome.cc @@ -24,6 +24,17 @@ namespace home { const char kGuestUserName[] = "$guest"; +// Path to user homes mounted with the mount_hidden option. The user home mount +// will be located at: +// kHiddenUserHomeBaseDir/<sanitized_user_name>/kHiddenUserHomeMountSubdir +const char kHiddenUserHomeBaseDir[] = "/home/.shadow"; +const char kHiddenUserHomeMountSubdir[] = "mount"; + +// Subdirectory of a user home mount where daemon-specific data is stored. +// This is used to assemble daemon data storage paths for hidden user home +// mounts. +const char kHiddenUserHomeRootSubdir[] = "root"; + static char g_user_home_prefix[PATH_MAX] = "/home/user/"; static char g_root_home_prefix[PATH_MAX] = "/home/root/"; static char g_system_salt_path[PATH_MAX] = "/home/.shadow/salt"; @@ -92,23 +103,35 @@ FilePath GetHashedUserPath(const std::string& hashed_username) { FilePath GetUserPath(const std::string& username) { if (!EnsureSystemSaltIsLoaded()) - return FilePath(""); + return FilePath(); return GetHashedUserPath(SanitizeUserName(username)); } FilePath GetRootPath(const std::string& username) { if (!EnsureSystemSaltIsLoaded()) - return FilePath(""); + return FilePath(); return FilePath(base::StringPrintf( "%s%s", g_root_home_prefix, SanitizeUserName(username).c_str())); } FilePath GetDaemonPath(const std::string& username, const std::string& daemon) { if (!EnsureSystemSaltIsLoaded()) - return FilePath(""); + return FilePath(); return GetRootPath(username).Append(daemon); } +FilePath GetDaemonPathForHiddenUserHome(const std::string& username, + const std::string& daemon) { + if (!EnsureSystemSaltIsLoaded()) + return FilePath(); + + return FilePath(kHiddenUserHomeBaseDir) + .Append(SanitizeUserName(username)) + .Append(kHiddenUserHomeMountSubdir) + .Append(kHiddenUserHomeRootSubdir) + .Append(daemon); +} + bool IsSanitizedUserName(const std::string& sanitized) { std::vector<uint8_t> bytes; return (sanitized.length() == 2 * SHA_DIGEST_LENGTH) && diff --git a/brillo/cryptohome.h b/brillo/cryptohome.h index caca31b..798d3a0 100644 --- a/brillo/cryptohome.h +++ b/brillo/cryptohome.h @@ -42,6 +42,12 @@ BRILLO_EXPORT base::FilePath GetRootPath(const std::string& username); BRILLO_EXPORT base::FilePath GetDaemonPath(const std::string& username, const std::string& daemon); +// Returns the path at which the daemon |daemon| should store per-user data +// when the user's home was mounted with mount_hidden. +BRILLO_EXPORT base::FilePath GetDaemonPathForHiddenUserHome( + const std::string& username, + const std::string& daemon); + // Checks whether |sanitized| has the format of a sanitized username. BRILLO_EXPORT bool IsSanitizedUserName(const std::string& sanitized); diff --git a/brillo/daemons/daemon.cc b/brillo/daemons/daemon.cc index 7c7e996..1b3d6d2 100644 --- a/brillo/daemons/daemon.cc +++ b/brillo/daemons/daemon.cc @@ -5,9 +5,9 @@ #include <brillo/daemons/daemon.h> #include <sysexits.h> +#include <time.h> #include <base/bind.h> -#include <base/files/file_path.h> #include <base/files/file_util.h> #include <base/logging.h> #include <base/run_loop.h> @@ -26,6 +26,8 @@ int Daemon::Run() { if (exit_code != EX_OK) return exit_code; + message_loop_.PostTask( + base::Bind(&Daemon::OnEventLoopStartedTask, base::Unretained(this))); message_loop_.Run(); OnShutdown(&exit_code_); @@ -68,6 +70,11 @@ int Daemon::OnInit() { return EX_OK; } +int Daemon::OnEventLoopStarted() { + // Do nothing. + return EX_OK; +} + void Daemon::OnShutdown(int* /* exit_code */) { // Do nothing. } @@ -89,4 +96,30 @@ bool Daemon::Restart(const signalfd_siginfo& /* info */) { return true; // Unregister the signal handler. } +void Daemon::OnEventLoopStartedTask() { + int exit_code = OnEventLoopStarted(); + if (exit_code != EX_OK) + QuitWithExitCode(exit_code); +} + +void UpdateLogSymlinks(const base::FilePath& latest_log_symlink, + const base::FilePath& previous_log_symlink, + const base::FilePath& log_file) { + base::DeleteFile(previous_log_symlink, false); + base::Move(latest_log_symlink, previous_log_symlink); + if (!base::CreateSymbolicLink(log_file.BaseName(), latest_log_symlink)) { + PLOG(ERROR) << "Unable to create symbolic link from " + << latest_log_symlink.value() << " to " << log_file.value(); + } +} + +std::string GetTimeAsLogString(const base::Time& time) { + time_t utime = time.ToTimeT(); + struct tm tm; + CHECK_EQ(localtime_r(&utime, &tm), &tm); + char str[16]; + CHECK_EQ(strftime(str, sizeof(str), "%Y%m%d-%H%M%S", &tm), 15UL); + return std::string(str); +} + } // namespace brillo diff --git a/brillo/daemons/daemon.h b/brillo/daemons/daemon.h index 8a11f0f..a16e04a 100644 --- a/brillo/daemons/daemon.h +++ b/brillo/daemons/daemon.h @@ -8,8 +8,10 @@ #include <string> #include <base/at_exit.h> +#include <base/files/file_path.h> #include <base/macros.h> #include <base/message_loop/message_loop.h> +#include <base/time/time.h> #include <brillo/asynchronous_signal_handler.h> #include <brillo/brillo_export.h> #include <brillo/message_loops/base_message_loop.h> @@ -66,6 +68,10 @@ class BRILLO_EXPORT Daemon : public AsynchronousSignalHandlerInterface { // is aborted and Daemon::Run() exits early. // When overloading, make sure you call the base implementation of OnInit(). virtual int OnInit(); + // Overload to provide initialization code that should be the first code to + // run on the message loop. Returning something other than EX_OK will cause + // the daemon to exit with that error code. + virtual int OnEventLoopStarted(); // Called when the message loops exits and before Daemon::Run() returns. // Overload to clean up the data that was set up during OnInit(). // |return_code| contains the current error code that will be returned from @@ -95,6 +101,9 @@ class BRILLO_EXPORT Daemon : public AsynchronousSignalHandlerInterface { // Called when SIGHUP signal is received. bool Restart(const signalfd_siginfo& info); + // Actual task posted first to the message loop. + void OnEventLoopStartedTask(); + // |at_exit_manager_| must be first to make sure it is initialized before // other members, especially the |message_loop_|. base::AtExitManager at_exit_manager_; @@ -109,6 +118,17 @@ class BRILLO_EXPORT Daemon : public AsynchronousSignalHandlerInterface { DISALLOW_COPY_AND_ASSIGN(Daemon); }; +// Moves |latest_log_symlink| to |previous_log_symlink| and creates a relative +// symlink at |latest_log_symlink| pointing to |log_file|. +// |latest_log_symlink| does not need to exist. +BRILLO_EXPORT void UpdateLogSymlinks(const base::FilePath& latest_log_symlink, + const base::FilePath& previous_log_symlink, + const base::FilePath& log_file); + +// Formats the current time with "%Y%m%d-%H%M%S". +// Commonly used for naming log files. +BRILLO_EXPORT std::string GetTimeAsLogString(const base::Time& time); + } // namespace brillo #endif // LIBBRILLO_BRILLO_DAEMONS_DAEMON_H_ diff --git a/brillo/dbus/data_serialization.cc b/brillo/dbus/data_serialization.cc index 746b241..4cae471 100644 --- a/brillo/dbus/data_serialization.cc +++ b/brillo/dbus/data_serialization.cc @@ -62,7 +62,7 @@ void AppendValueToWriter(dbus::MessageWriter* writer, } void AppendValueToWriter(dbus::MessageWriter* writer, - const base::ScopedFD& value) { + const FileDescriptor& value) { writer->AppendFileDescriptor(value.get()); } @@ -142,9 +142,8 @@ bool PopValueFromReader(dbus::MessageReader* reader, dbus::ObjectPath* value) { bool PopValueFromReader(dbus::MessageReader* reader, base::ScopedFD* value) { dbus::MessageReader variant_reader(nullptr); - bool ok = details::DescendIntoVariantIfPresent(&reader, &variant_reader) && - reader->PopFileDescriptor(value); - return ok; + return details::DescendIntoVariantIfPresent(&reader, &variant_reader) && + reader->PopFileDescriptor(value); } namespace { @@ -307,7 +306,7 @@ bool PopValueFromReader(dbus::MessageReader* reader, brillo::Any* value) { return false; case dbus::Message::UNIX_FD: CHECK(dbus::IsDBusTypeUnixFdSupported()) << "UNIX_FD data not supported"; - // base::ScopedFD is not a copyable type. Cannot be returned via + // File descriptors don't use copyable types. Cannot be returned via // brillo::Any. Fail here. LOG(ERROR) << "Cannot return FileDescriptor via Any"; return false; diff --git a/brillo/dbus/data_serialization.h b/brillo/dbus/data_serialization.h index 6919b19..1600919 100644 --- a/brillo/dbus/data_serialization.h +++ b/brillo/dbus/data_serialization.h @@ -36,7 +36,8 @@ // | (UVW...) | std::tuple<U,V,W,...> // DICT | a{KV} | std::map<K,V> // VARIANT | v | brillo::Any -// UNIX_FD | h | base::ScopedFD +// UNIX_FD | h | brillo::dbus_utils::FileDescriptor (write) +// | | base::ScopedFD (read) // SIGNATURE | g | (unsupported) // // Additional overloads/specialization can be provided for custom types. @@ -57,9 +58,11 @@ #include <utility> #include <vector> +#include <base/files/scoped_file.h> #include <base/logging.h> #include <base/files/scoped_file.h> #include <brillo/brillo_export.h> +#include <brillo/dbus/file_descriptor.h> #include <brillo/type_name_undecorate.h> #include <dbus/message.h> @@ -412,21 +415,28 @@ struct DBusType<dbus::ObjectPath> { } }; -// base::ScopedFD ------------------------------------------------------------- +// brillo::dbus_utils::FileDescriptor/base::ScopedFD -------------------------- BRILLO_EXPORT void AppendValueToWriter(dbus::MessageWriter* writer, - const base::ScopedFD& value); + const FileDescriptor& value); BRILLO_EXPORT bool PopValueFromReader(dbus::MessageReader* reader, base::ScopedFD* value); template<> -struct DBusType<base::ScopedFD> { +struct DBusType<FileDescriptor> { inline static std::string GetSignature() { return DBUS_TYPE_UNIX_FD_AS_STRING; } inline static void Write(dbus::MessageWriter* writer, - const base::ScopedFD& value) { + const FileDescriptor& value) { AppendValueToWriter(writer, value); } +}; + +template<> +struct DBusType<base::ScopedFD> { + inline static std::string GetSignature() { + return DBUS_TYPE_UNIX_FD_AS_STRING; + } inline static bool Read(dbus::MessageReader* reader, base::ScopedFD* value) { return PopValueFromReader(reader, value); diff --git a/brillo/dbus/data_serialization_unittest.cc b/brillo/dbus/data_serialization_unittest.cc index 8771fe9..c7d5e0f 100644 --- a/brillo/dbus/data_serialization_unittest.cc +++ b/brillo/dbus/data_serialization_unittest.cc @@ -12,7 +12,6 @@ #include "brillo/dbus/test.pb.h" -using base::ScopedFD; using dbus::Message; using dbus::MessageReader; using dbus::MessageWriter; @@ -34,7 +33,8 @@ TEST(DBusUtils, Supported_BasicTypes) { EXPECT_TRUE(IsTypeSupported<double>::value); EXPECT_TRUE(IsTypeSupported<std::string>::value); EXPECT_TRUE(IsTypeSupported<ObjectPath>::value); - EXPECT_TRUE(IsTypeSupported<ScopedFD>::value); + EXPECT_TRUE(IsTypeSupported<FileDescriptor>::value); + EXPECT_TRUE(IsTypeSupported<base::ScopedFD>::value); EXPECT_TRUE(IsTypeSupported<Any>::value); EXPECT_TRUE(IsTypeSupported<google::protobuf::MessageLite>::value); EXPECT_TRUE(IsTypeSupported<dbus_utils_test::TestMessage>::value); @@ -90,7 +90,8 @@ TEST(DBusUtils, Signatures_BasicTypes) { EXPECT_EQ("d", GetDBusSignature<double>()); EXPECT_EQ("s", GetDBusSignature<std::string>()); EXPECT_EQ("o", GetDBusSignature<ObjectPath>()); - EXPECT_EQ("h", GetDBusSignature<ScopedFD>()); + EXPECT_EQ("h", GetDBusSignature<FileDescriptor>()); + EXPECT_EQ("h", GetDBusSignature<base::ScopedFD>()); EXPECT_EQ("v", GetDBusSignature<Any>()); } @@ -106,7 +107,8 @@ TEST(DBusUtils, Signatures_Arrays) { EXPECT_EQ("ad", GetDBusSignature<std::vector<double>>()); EXPECT_EQ("as", GetDBusSignature<std::vector<std::string>>()); EXPECT_EQ("ao", GetDBusSignature<std::vector<ObjectPath>>()); - EXPECT_EQ("ah", GetDBusSignature<std::vector<ScopedFD>>()); + EXPECT_EQ("ah", GetDBusSignature<std::vector<FileDescriptor>>()); + EXPECT_EQ("ah", GetDBusSignature<std::vector<base::ScopedFD>>()); EXPECT_EQ("av", GetDBusSignature<std::vector<Any>>()); EXPECT_EQ("a(is)", (GetDBusSignature<std::vector<std::pair<int, std::string>>>())); @@ -239,24 +241,17 @@ TEST(DBusUtils, AppendAndPopFileDescriptor) { MessageWriter writer(message.get()); // Append stdout. - ScopedFD temp(1); - // Descriptor should not be valid until checked. - EXPECT_FALSE(temp.is_valid()); - // NB: thread IO requirements not relevant for unit tests. - temp.CheckValidity(); - EXPECT_TRUE(temp.is_valid()); + FileDescriptor temp = 1; AppendValueToWriter(&writer, temp); EXPECT_EQ("h", message->GetSignature()); - ScopedFD fd_value; + base::ScopedFD fd_value; MessageReader reader(message.get()); EXPECT_TRUE(reader.HasMoreData()); EXPECT_TRUE(PopValueFromReader(&reader, &fd_value)); EXPECT_FALSE(reader.HasMoreData()); - // Descriptor is automatically checked for validity as part of - // PopValueFromReader() call. EXPECT_TRUE(fd_value.is_valid()); } diff --git a/brillo/dbus/dbus_method_invoker.h b/brillo/dbus/dbus_method_invoker.h index 2a8708b..f8b6990 100644 --- a/brillo/dbus/dbus_method_invoker.h +++ b/brillo/dbus/dbus_method_invoker.h @@ -67,8 +67,10 @@ #include <tuple> #include <base/bind.h> +#include <base/files/scoped_file.h> #include <brillo/dbus/dbus_param_reader.h> #include <brillo/dbus/dbus_param_writer.h> +#include <brillo/dbus/file_descriptor.h> #include <brillo/dbus/utils.h> #include <brillo/errors/error.h> #include <brillo/errors/error_codes.h> @@ -140,6 +142,30 @@ inline std::unique_ptr<dbus::Response> CallMethodAndBlock( args...); } +namespace internal { +// In order to support non-copyable file descriptor types, we have this +// internal::HackMove() helper function that does really nothing for normal +// types but uses Pass() for file descriptors so we can move them out from +// the temporaries created inside DBusParamReader<...>::Invoke(). +// If only libchrome supported real rvalues so we can just do std::move() and +// be done with it. +template <typename T> +inline const T& HackMove(const T& val) { + return val; +} + +// Even though |val| here is passed as const&, the actual value is created +// inside DBusParamReader<...>::Invoke() and is temporary in nature, so it is +// safe to move the file descriptor out of |val|. That's why we are doing +// const_cast here. It is a bit hacky, but there is no negative side effects. +inline base::ScopedFD HackMove(const base::ScopedFD& val) { + return std::move(const_cast<base::ScopedFD&>(val)); +} +inline FileDescriptor HackMove(const FileDescriptor& val) { + return std::move(const_cast<FileDescriptor&>(val)); +} +} // namespace internal + // Extracts the parameters of |ResultTypes...| types from the message reader // and puts the values in the resulting |tuple|. Returns false on error and // provides additional error details in |error| object. diff --git a/brillo/dbus/dbus_method_invoker_unittest.cc b/brillo/dbus/dbus_method_invoker_unittest.cc index dfd10c4..34f4c6f 100644 --- a/brillo/dbus/dbus_method_invoker_unittest.cc +++ b/brillo/dbus/dbus_method_invoker_unittest.cc @@ -26,6 +26,37 @@ using dbus::MessageReader; using dbus::MessageWriter; using dbus::Response; +namespace { + +void SuccessCallback(const std::string& expected_result, + int* counter, + const std::string& actual_result) { + (*counter)++; + EXPECT_EQ(expected_result, actual_result); +} + +void SimpleSuccessCallback(int* counter, const std::string& result) { + (*counter)++; +} + +void ErrorCallback(const std::string& domain, + const std::string& code, + const std::string& message, + int* counter, + brillo::Error* error) { + (*counter)++; + ASSERT_NE(nullptr, error); + EXPECT_EQ(domain, error->GetDomain()); + EXPECT_EQ(code, error->GetCode()); + EXPECT_EQ(message, error->GetMessage()); +} + +void SimpleErrorCallback(int* counter, brillo::Error* error) { + (*counter)++; +} + +} // namespace + namespace brillo { namespace dbus_utils { @@ -95,8 +126,7 @@ class DBusMethodInvokerTest : public testing::Test { if (reader.PopFileDescriptor(&fd)) { auto response = Response::CreateEmpty(); MessageWriter writer(response.get()); - fd.CheckValidity(); - writer.AppendFileDescriptor(fd); + writer.AppendFileDescriptor(fd.get()); return response.release(); } } @@ -131,19 +161,19 @@ class DBusMethodInvokerTest : public testing::Test { return result; } - // Sends a file descriptor received over D-Bus back to the caller. - base::ScopedFD EchoFD(const base::ScopedFD& fd_in) { + // Sends a file descriptor received over D-Bus back to the caller using the + // new types. + base::ScopedFD EchoFD(int fd_in) { std::unique_ptr<dbus::Response> response = - brillo::dbus_utils::CallMethodAndBlock(mock_object_proxy_.get(), - kTestInterface, kTestMethod4, - nullptr, fd_in); + brillo::dbus_utils::CallMethodAndBlock( + mock_object_proxy_.get(), kTestInterface, kTestMethod4, + nullptr, brillo::dbus_utils::FileDescriptor{fd_in}); EXPECT_NE(nullptr, response.get()); base::ScopedFD fd_out; using brillo::dbus_utils::ExtractMethodCallResults; EXPECT_TRUE(ExtractMethodCallResults(response.get(), nullptr, &fd_out)); return fd_out; } - scoped_refptr<dbus::MockBus> bus_; scoped_refptr<dbus::MockObjectProxy> mock_object_proxy_; }; @@ -180,15 +210,18 @@ TEST_F(DBusMethodInvokerTest, TestFileDescriptors) { // Passing a file descriptor over D-Bus would effectively duplicate the fd. // So the resulting file descriptor value would be different but it still // should be valid. - base::ScopedFD fd_stdin(0); - fd_stdin.CheckValidity(); - EXPECT_NE(fd_stdin.value(), EchoFD(fd_stdin).value()); - base::ScopedFD fd_stdout(1); - fd_stdout.CheckValidity(); - EXPECT_NE(fd_stdout.value(), EchoFD(fd_stdout).value()); - base::ScopedFD fd_stderr(2); - fd_stderr.CheckValidity(); - EXPECT_NE(fd_stderr.value(), EchoFD(fd_stderr).value()); + int fd_stdin = 0; + base::ScopedFD out_fd = EchoFD(fd_stdin); + EXPECT_NE(fd_stdin, out_fd.get()); + EXPECT_TRUE(out_fd.is_valid()); + int fd_stdout = 1; + out_fd = EchoFD(fd_stdout); + EXPECT_NE(fd_stdout, out_fd.get()); + EXPECT_TRUE(out_fd.is_valid()); + int fd_stderr = 2; + out_fd = EchoFD(fd_stderr); + EXPECT_NE(fd_stderr, out_fd.get()); + EXPECT_TRUE(out_fd.is_valid()); } ////////////////////////////////////////////////////////////////////////////// @@ -246,62 +279,6 @@ class AsyncDBusMethodInvokerTest : public testing::Test { LOG(FATAL) << "Unexpected method call: " << method_call->ToString(); } - base::Callback<void(const std::string&)> SuccessCallback( - const std::string& in_result, int* in_counter) { - return base::Bind( - [](const std::string& result, - int* counter, - const std::string& actual_result) { - (*counter)++; - EXPECT_EQ(result, actual_result); - }, - in_result, - base::Unretained(in_counter)); - } - - base::Callback<void(const std::string&)> SuccessCallback(int* in_counter) { - return base::Bind( - [](int* counter, const std::string& actual_result) { - (*counter)++; - EXPECT_EQ("", actual_result); - }, - base::Unretained(in_counter)); - } - - AsyncErrorCallback ErrorCallback(int* in_counter) { - return base::Bind( - [](int* counter, brillo::Error* error) { - (*counter)++; - EXPECT_NE(nullptr, error); - EXPECT_EQ("", error->GetDomain()); - EXPECT_EQ("", error->GetCode()); - EXPECT_EQ("", error->GetMessage()); - }, - base::Unretained(in_counter)); - } - - AsyncErrorCallback ErrorCallback(const std::string& domain, - const std::string& code, - const std::string& message, - int* in_counter) { - return base::Bind( - [](const std::string& domain, - const std::string& code, - const std::string& message, - int* counter, - brillo::Error* error) { - (*counter)++; - EXPECT_NE(nullptr, error); - EXPECT_EQ(domain, error->GetDomain()); - EXPECT_EQ(code, error->GetCode()); - EXPECT_EQ(message, error->GetMessage()); - }, - domain, - code, - message, - base::Unretained(in_counter)); - } - scoped_refptr<dbus::MockBus> bus_; scoped_refptr<dbus::MockObjectProxy> mock_object_proxy_; }; @@ -313,22 +290,22 @@ TEST_F(AsyncDBusMethodInvokerTest, TestSuccess) { mock_object_proxy_.get(), kTestInterface, kTestMethod1, - base::Bind(SuccessCallback("4", &success_count)), - base::Bind(ErrorCallback(&error_count)), + base::Bind(SuccessCallback, "4", &success_count), + base::Bind(SimpleErrorCallback, &error_count), 2, 2); brillo::dbus_utils::CallMethod( mock_object_proxy_.get(), kTestInterface, kTestMethod1, - base::Bind(SuccessCallback("10", &success_count)), - base::Bind(ErrorCallback(&error_count)), + base::Bind(SuccessCallback, "10", &success_count), + base::Bind(SimpleErrorCallback, &error_count), 3, 7); brillo::dbus_utils::CallMethod( mock_object_proxy_.get(), kTestInterface, kTestMethod1, - base::Bind(SuccessCallback("-4", &success_count)), - base::Bind(ErrorCallback(&error_count)), + base::Bind(SuccessCallback, "-4", &success_count), + base::Bind(SimpleErrorCallback, &error_count), 13, -17); EXPECT_EQ(0, error_count); EXPECT_EQ(3, success_count); @@ -341,11 +318,12 @@ TEST_F(AsyncDBusMethodInvokerTest, TestFailure) { mock_object_proxy_.get(), kTestInterface, kTestMethod2, - base::Bind(SuccessCallback(&success_count)), - base::Bind(ErrorCallback(brillo::errors::dbus::kDomain, - "org.MyError", - "My error message", - &error_count)), + base::Bind(SimpleSuccessCallback, &success_count), + base::Bind(ErrorCallback, + brillo::errors::dbus::kDomain, + "org.MyError", + "My error message", + &error_count), 2, 2); EXPECT_EQ(1, error_count); EXPECT_EQ(0, success_count); diff --git a/brillo/dbus/dbus_object.cc b/brillo/dbus/dbus_object.cc index 59350a5..512cd6f 100644 --- a/brillo/dbus/dbus_object.cc +++ b/brillo/dbus/dbus_object.cc @@ -16,6 +16,23 @@ namespace brillo { namespace dbus_utils { +namespace { + +void SetupDefaultPropertyHandlers(DBusInterface* prop_interface, + ExportedPropertySet* property_set) { + prop_interface->AddSimpleMethodHandler(dbus::kPropertiesGetAll, + base::Unretained(property_set), + &ExportedPropertySet::HandleGetAll); + prop_interface->AddSimpleMethodHandlerWithError( + dbus::kPropertiesGet, base::Unretained(property_set), + &ExportedPropertySet::HandleGet); + prop_interface->AddSimpleMethodHandlerWithError( + dbus::kPropertiesSet, base::Unretained(property_set), + &ExportedPropertySet::HandleSet); +} + +} // namespace + ////////////////////////////////////////////////////////////////////////////// DBusInterface::DBusInterface(DBusObject* dbus_object, @@ -29,6 +46,11 @@ void DBusInterface::AddProperty(const std::string& property_name, interface_name_, property_name, prop_base); } +void DBusInterface::RemoveProperty(const std::string& property_name) { + dbus_object_->property_set_.UnregisterProperty(interface_name_, + property_name); +} + void DBusInterface::ExportAsync( ExportedObjectManager* object_manager, dbus::Bus* /* bus */, @@ -152,7 +174,21 @@ void DBusInterface::AddSignalImpl( DBusObject::DBusObject(ExportedObjectManager* object_manager, const scoped_refptr<dbus::Bus>& bus, const dbus::ObjectPath& object_path) - : property_set_(bus.get()), bus_(bus), object_path_(object_path) { + : DBusObject::DBusObject(object_manager, + bus, + object_path, + base::Bind(&SetupDefaultPropertyHandlers)) {} + +DBusObject::DBusObject( + ExportedObjectManager* object_manager, + const scoped_refptr<dbus::Bus>& bus, + const dbus::ObjectPath& object_path, + PropertyHandlerSetupCallback property_handler_setup_callback) + : property_set_(bus.get()), + bus_(bus), + object_path_(object_path), + property_handler_setup_callback_( + std::move(property_handler_setup_callback)) { if (object_manager) object_manager_ = object_manager->AsWeakPtr(); } @@ -183,6 +219,21 @@ DBusInterface* DBusObject::FindInterface( return (itf_iter == interfaces_.end()) ? nullptr : itf_iter->second.get(); } +void DBusObject::RemoveInterface(const std::string& interface_name) { + auto itf_iter = interfaces_.find(interface_name); + CHECK(itf_iter != interfaces_.end()) + << "Interface " << interface_name << " has not been added."; + interfaces_.erase(itf_iter); +} + +void DBusObject::ExportInterfaceAsync( + const std::string& interface_name, + const AsyncEventSequencer::CompletionAction& completion_callback) { + AddOrGetInterface(interface_name) + ->ExportAsync(object_manager_.get(), bus_.get(), exported_object_, + object_path_, completion_callback); +} + void DBusObject::RegisterAsync( const AsyncEventSequencer::CompletionAction& completion_callback) { VLOG(1) << "Registering D-Bus object '" << object_path_.value() << "'."; @@ -190,21 +241,7 @@ void DBusObject::RegisterAsync( scoped_refptr<AsyncEventSequencer> sequencer(new AsyncEventSequencer()); exported_object_ = bus_->GetExportedObject(object_path_); - // Add the org.freedesktop.DBus.Properties interface to the object. - DBusInterface* prop_interface = AddOrGetInterface(dbus::kPropertiesInterface); - prop_interface->AddSimpleMethodHandler( - dbus::kPropertiesGetAll, - base::Unretained(&property_set_), - &ExportedPropertySet::HandleGetAll); - prop_interface->AddSimpleMethodHandlerWithError( - dbus::kPropertiesGet, - base::Unretained(&property_set_), - &ExportedPropertySet::HandleGet); - prop_interface->AddSimpleMethodHandlerWithError( - dbus::kPropertiesSet, - base::Unretained(&property_set_), - &ExportedPropertySet::HandleSet); - property_set_.OnPropertiesInterfaceExported(prop_interface); + RegisterPropertiesInterface(); // Export interface methods for (const auto& pair : interfaces_) { @@ -225,21 +262,7 @@ void DBusObject::RegisterAndBlock() { CHECK(exported_object_ == nullptr) << "Object already registered."; exported_object_ = bus_->GetExportedObject(object_path_); - // Add the org.freedesktop.DBus.Properties interface to the object. - DBusInterface* prop_interface = AddOrGetInterface(dbus::kPropertiesInterface); - prop_interface->AddSimpleMethodHandler( - dbus::kPropertiesGetAll, - base::Unretained(&property_set_), - &ExportedPropertySet::HandleGetAll); - prop_interface->AddSimpleMethodHandlerWithError( - dbus::kPropertiesGet, - base::Unretained(&property_set_), - &ExportedPropertySet::HandleGet); - prop_interface->AddSimpleMethodHandlerWithError( - dbus::kPropertiesSet, - base::Unretained(&property_set_), - &ExportedPropertySet::HandleSet); - property_set_.OnPropertiesInterfaceExported(prop_interface); + RegisterPropertiesInterface(); // Export interface methods for (const auto& pair : interfaces_) { @@ -275,5 +298,11 @@ bool DBusObject::SendSignal(dbus::Signal* signal) { return false; } +void DBusObject::RegisterPropertiesInterface() { + DBusInterface* prop_interface = AddOrGetInterface(dbus::kPropertiesInterface); + property_handler_setup_callback_.Run(prop_interface, &property_set_); + property_set_.OnPropertiesInterfaceExported(prop_interface); +} + } // namespace dbus_utils } // namespace brillo diff --git a/brillo/dbus/dbus_object.h b/brillo/dbus/dbus_object.h index 6cd34dd..61c954f 100644 --- a/brillo/dbus/dbus_object.h +++ b/brillo/dbus/dbus_object.h @@ -372,6 +372,9 @@ class BRILLO_EXPORT DBusInterface final { void AddProperty(const std::string& property_name, ExportedPropertyBase* prop_base); + // Unregisters a D-Bus property. + void RemoveProperty(const std::string& property_name); + // Registers a D-Bus signal that has a specified number and types (|Args|) of // arguments. Returns a weak pointer to the DBusSignal object which can be // used to send the signal on this interface when needed: @@ -507,6 +510,9 @@ class BRILLO_EXPORT DBusInterface final { // by this object. class BRILLO_EXPORT DBusObject { public: + using PropertyHandlerSetupCallback = base::Callback<void( + DBusInterface* prop_interface, ExportedPropertySet* property_set)>; + // object_manager - ExportedObjectManager instance that notifies D-Bus // listeners of a new interface being claimed and property // changes on those interfaces. @@ -514,6 +520,17 @@ class BRILLO_EXPORT DBusObject { DBusObject(ExportedObjectManager* object_manager, const scoped_refptr<dbus::Bus>& bus, const dbus::ObjectPath& object_path); + + // property_handler_setup_callback - To be called when setting up property + // method handlers. Clients can register + // their own custom property method handlers + // (GetAll/Get/Set) by passing in this + // callback. + DBusObject(ExportedObjectManager* object_manager, + const scoped_refptr<dbus::Bus>& bus, + const dbus::ObjectPath& object_path, + PropertyHandlerSetupCallback property_handler_setup_callback); + virtual ~DBusObject(); // Returns an proxy handler for the interface |interface_name|. If the @@ -524,6 +541,16 @@ class BRILLO_EXPORT DBusObject { // interface registered by this name. DBusInterface* FindInterface(const std::string& interface_name) const; + // Removes the previously added proxy handler for the interface + // |interface_name|. + void RemoveInterface(const std::string& interface_name); + + // Exports a proxy handler for the interface |interface_name|. If the + // interface proxy does not exist yet, it will be automatically created. + void ExportInterfaceAsync( + const std::string& interface_name, + const AsyncEventSequencer::CompletionAction& completion_callback); + // Registers the object instance with D-Bus. This is an asynchronous call // that will call |completion_callback| when the object and all of its // interfaces are registered. @@ -555,6 +582,9 @@ class BRILLO_EXPORT DBusObject { scoped_refptr<dbus::Bus> GetBus() { return bus_; } private: + // Add the org.freedesktop.DBus.Properties interface to the object. + void RegisterPropertiesInterface(); + // A map of all the interfaces added to this object. std::map<std::string, std::unique_ptr<DBusInterface>> interfaces_; // Exported property set for properties registered with the interfaces @@ -568,6 +598,8 @@ class BRILLO_EXPORT DBusObject { dbus::ObjectPath object_path_; // D-Bus object instance once this object is successfully exported. dbus::ExportedObject* exported_object_ = nullptr; // weak; owned by |bus_|. + // Sets up property method handlers. + PropertyHandlerSetupCallback property_handler_setup_callback_; friend class DBusInterface; DISALLOW_COPY_AND_ASSIGN(DBusObject); diff --git a/brillo/dbus/dbus_object_unittest.cc b/brillo/dbus/dbus_object_unittest.cc index eaec3d8..932a5c8 100644 --- a/brillo/dbus/dbus_object_unittest.cc +++ b/brillo/dbus/dbus_object_unittest.cc @@ -43,6 +43,8 @@ const char kTestMethod_NoOp[] = "NoOp"; const char kTestMethod_WithMessage[] = "TestWithMessage"; const char kTestMethod_WithMessageAsync[] = "TestWithMessageAsync"; +const char kTestInterface4[] = "org.chromium.Test.LateInterface"; + struct Calc { int Add(int x, int y) { return x + y; } int Negate(int x) { return -x; } @@ -89,6 +91,10 @@ void TestWithMessageAsync( response->Return(message->GetSender()); } +void OnInterfaceExported(bool success) { + // Does nothing. +} + } // namespace class DBusObjectTest : public ::testing::Test { @@ -315,6 +321,35 @@ TEST_F(DBusObjectTest, TestWithMessageAsync) { EXPECT_EQ(sender, message); } +TEST_F(DBusObjectTest, TestRemovedInterface) { + // Removes the interface to be tested. + dbus_object_->RemoveInterface(kTestInterface3); + + const std::string sender{":1.2345"}; + dbus::MethodCall method_call(kTestInterface3, kTestMethod_WithMessage); + method_call.SetSerial(123); + method_call.SetSender(sender); + auto response = testing::CallMethod(*dbus_object_, &method_call); + // The response should contain error UnknownInterface since the interface has + // been intentionally removed. + EXPECT_EQ(DBUS_ERROR_UNKNOWN_INTERFACE, response->GetErrorName()); +} + +TEST_F(DBusObjectTest, TestInterfaceExportedLate) { + // Registers a new interface late. + dbus_object_->ExportInterfaceAsync(kTestInterface4, + base::Bind(&OnInterfaceExported)); + + const std::string sender{":1.2345"}; + dbus::MethodCall method_call(kTestInterface4, kTestMethod_WithMessage); + method_call.SetSerial(123); + method_call.SetSender(sender); + auto response = testing::CallMethod(*dbus_object_, &method_call); + // The response should contain error UnknownMethod rather than + // UnknownInterface since the interface has been registered late. + EXPECT_EQ(DBUS_ERROR_UNKNOWN_METHOD, response->GetErrorName()); +} + TEST_F(DBusObjectTest, TooFewParams) { dbus::MethodCall method_call(kTestInterface1, kTestMethod_Add); method_call.SetSerial(123); diff --git a/brillo/dbus/dbus_signal_handler.h b/brillo/dbus/dbus_signal_handler.h index 9c3246c..15cdae1 100644 --- a/brillo/dbus/dbus_signal_handler.h +++ b/brillo/dbus/dbus_signal_handler.h @@ -5,6 +5,7 @@ #ifndef LIBBRILLO_BRILLO_DBUS_DBUS_SIGNAL_HANDLER_H_ #define LIBBRILLO_BRILLO_DBUS_DBUS_SIGNAL_HANDLER_H_ +#include <functional> #include <string> #include <brillo/bind_lambda.h> @@ -58,10 +59,11 @@ void ConnectToSignal( }; // Register our stub handler with D-Bus ObjectProxy. - object_proxy->ConnectToSignal(interface_name, - signal_name, - base::Bind(dbus_signal_callback, signal_callback), - on_connected_callback); + object_proxy->ConnectToSignal( + interface_name, + signal_name, + base::Bind(dbus_signal_callback, signal_callback), + on_connected_callback); } } // namespace dbus_utils diff --git a/brillo/dbus/exported_object_manager.h b/brillo/dbus/exported_object_manager.h index 01dab5b..ea68f33 100644 --- a/brillo/dbus/exported_object_manager.h +++ b/brillo/dbus/exported_object_manager.h @@ -108,6 +108,13 @@ class BRILLO_EXPORT ExportedObjectManager const scoped_refptr<dbus::Bus>& GetBus() const { return bus_; } + // Due to D-Bus forwarding, clients may need to access the underlying + // DBusObject to handle signals/methods. + // TODO(sonnysasaka): Refactor this accessor into a stricter API once we know + // what D-Bus forwarding needs when it's completed, without exposing + // DBusObject directly. + brillo::dbus_utils::DBusObject* dbus_object() { return &dbus_object_; }; + private: BRILLO_PRIVATE ObjectMap HandleGetManagedObjects(); diff --git a/brillo/dbus/exported_property_set.cc b/brillo/dbus/exported_property_set.cc index 8d6ae65..018843e 100644 --- a/brillo/dbus/exported_property_set.cc +++ b/brillo/dbus/exported_property_set.cc @@ -54,6 +54,17 @@ void ExportedPropertySet::RegisterProperty( exported_property->SetUpdateCallback(cb); } +void ExportedPropertySet::UnregisterProperty(const std::string& interface_name, + const std::string& property_name) { + bus_->AssertOnOriginThread(); + auto& prop_map = properties_[interface_name]; + auto prop_iter = prop_map.find(property_name); + CHECK(prop_iter != prop_map.end()) + << "Property '" << property_name << "' doesn't exist"; + prop_iter->second->ClearUpdateCallback(); + prop_map.erase(prop_iter); +} + VariantDictionary ExportedPropertySet::HandleGetAll( const std::string& interface_name) { bus_->AssertOnOriginThread(); @@ -160,6 +171,10 @@ void ExportedPropertyBase::SetUpdateCallback(const OnUpdateCallback& cb) { on_update_callback_ = cb; } +void ExportedPropertyBase::ClearUpdateCallback() { + on_update_callback_.Reset(); +} + void ExportedPropertyBase::SetAccessMode( ExportedPropertyBase::Access access_mode) { access_mode_ = access_mode; diff --git a/brillo/dbus/exported_property_set.h b/brillo/dbus/exported_property_set.h index f10511f..971e932 100644 --- a/brillo/dbus/exported_property_set.h +++ b/brillo/dbus/exported_property_set.h @@ -71,6 +71,9 @@ class BRILLO_EXPORT ExportedPropertyBase { // interface of the exported object. virtual void SetUpdateCallback(const OnUpdateCallback& cb); + // Clears the update callback that was previously set with SetUpdateCallback. + virtual void ClearUpdateCallback(); + // Returns the contained value as Any. virtual brillo::Any GetValue() const = 0; @@ -111,6 +114,10 @@ class BRILLO_EXPORT ExportedPropertySet { const std::string& property_name, ExportedPropertyBase* exported_property); + // Unregisters a property from this exported property set. + void UnregisterProperty(const std::string& interface_name, + const std::string& property_name); + // D-Bus methods for org.freedesktop.DBus.Properties interface. VariantDictionary HandleGetAll(const std::string& interface_name); bool HandleGet(brillo::ErrorPtr* error, @@ -208,7 +215,7 @@ class ExportedProperty : public ExportedPropertyBase { if (!validator_.is_null() && !validator_.Run(error, value.Get<T>())) { return false; } - value_ = value.Get<T>(); + SetValue(value.Get<T>()); return true; } diff --git a/brillo/dbus/exported_property_set_unittest.cc b/brillo/dbus/exported_property_set_unittest.cc index 9233b4a..93aceb4 100644 --- a/brillo/dbus/exported_property_set_unittest.cc +++ b/brillo/dbus/exported_property_set_unittest.cc @@ -336,6 +336,18 @@ TEST_F(ExportedPropertySetTest, GetExtraArgs) { AssertMethodReturnsError(&method_call); } +TEST_F(ExportedPropertySetTest, GetRemovedProperty) { + DBusInterface* itf1 = p_->dbus_object_.AddOrGetInterface(kTestInterface1); + itf1->RemoveProperty(kBoolPropName); + + auto response = GetPropertyOnInterface(kTestInterface1, kBoolPropName); + ASSERT_EQ(DBUS_ERROR_UNKNOWN_PROPERTY, response->GetErrorName()); + + // Signal should not be emitted for removed property. + EXPECT_CALL(*mock_exported_object_, SendSignal(_)).Times(0); + p_->bool_prop_.SetValue(true); +} + TEST_F(ExportedPropertySetTest, GetWorksWithBool) { auto response = GetPropertyOnInterface(kTestInterface1, kBoolPropName); dbus::MessageReader reader(response.get()); diff --git a/brillo/dbus/file_descriptor.h b/brillo/dbus/file_descriptor.h new file mode 100644 index 0000000..f7be44f --- /dev/null +++ b/brillo/dbus/file_descriptor.h @@ -0,0 +1,57 @@ +// Copyright 2018 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef LIBBRILLO_BRILLO_DBUS_FILE_DESCRIPTOR_H_ +#define LIBBRILLO_BRILLO_DBUS_FILE_DESCRIPTOR_H_ + +#include <base/files/scoped_file.h> +#include <base/macros.h> + +namespace brillo { +namespace dbus_utils { + +// This struct wraps file descriptors to give them a type other than int. +// Implicit conversions are provided because this should be as transparent +// a wrapper as possible to match the libchrome bindings below when this +// class is used by chromeos-dbus-bindings. +// +// Because we might pass these around and the calling code neither passes +// ownership nor knows when this will be destroyed, it actually dups the FD +// so that the calling code and binding code both have a clear handle on the +// lifetimes of their respective copies of the FD. +struct FileDescriptor { + FileDescriptor() = default; + FileDescriptor(int fd) : fd(dup(fd)) {} + FileDescriptor(FileDescriptor&& other) : fd(std::move(other.fd)) {} + FileDescriptor(base::ScopedFD&& other) : fd(std::move(other)) {} + + inline FileDescriptor& operator=(int new_fd) { + fd.reset(dup(new_fd)); + return *this; + } + + FileDescriptor& operator=(FileDescriptor&& other) { + fd = std::move(other.fd); + return *this; + } + + FileDescriptor& operator=(base::ScopedFD&& other) { + fd = std::move(other); + return *this; + } + + int release() { return fd.release(); } + + int get() const { return fd.get(); } + + private: + DISALLOW_COPY_AND_ASSIGN(FileDescriptor); + + base::ScopedFD fd; +}; + +} // namespace dbus_utils +} // namespace brillo + +#endif // LIBBRILLO_BRILLO_DBUS_FILE_DESCRIPTOR_H_ diff --git a/brillo/file_utils.cc b/brillo/file_utils.cc index 24ed347..8faa1b7 100644 --- a/brillo/file_utils.cc +++ b/brillo/file_utils.cc @@ -12,11 +12,17 @@ #include <base/files/scoped_file.h> #include <base/logging.h> #include <base/posix/eintr_wrapper.h> +#include <base/rand_util.h> +#include <base/strings/string_number_conversions.h> +#include <base/time/time.h> namespace brillo { namespace { +// Log sync(), fsync(), etc. calls that take this many seconds or longer. +constexpr const base::TimeDelta kLongSync = base::TimeDelta::FromSeconds(10); + enum { kPermissions600 = S_IRUSR | S_IWUSR, kPermissions777 = S_IRWXU | S_IRWXG | S_IRWXO @@ -112,11 +118,9 @@ bool TouchFileInternal(const base::FilePath& path, } // Create the file as owner-only initially. - base::ScopedFD scoped_fd( - HANDLE_EINTR(openat(AT_FDCWD, - path.value().c_str(), - O_RDONLY | O_NOFOLLOW | O_CREAT | O_EXCL | O_CLOEXEC, - kPermissions600))); + base::ScopedFD scoped_fd(HANDLE_EINTR(openat( + AT_FDCWD, path.value().c_str(), + O_RDONLY | O_NOFOLLOW | O_CREAT | O_EXCL | O_CLOEXEC, kPermissions600))); if (scoped_fd == -1) { PLOG(WARNING) << "Failed to create file \"" << path.value() << '"'; return false; @@ -128,6 +132,24 @@ bool TouchFileInternal(const base::FilePath& path, return true; } +std::string GetRandomSuffix() { + const int kBufferSize = 6; + unsigned char buffer[kBufferSize]; + base::RandBytes(buffer, arraysize(buffer)); + std::string suffix; + for (int i = 0; i < kBufferSize; ++i) { + int random_value = buffer[i] % (2 * 26 + 10); + if (random_value < 26) { + suffix.push_back('a' + random_value); + } else if (random_value < 2 * 26) { + suffix.push_back('A' + random_value - 26); + } else { + suffix.push_back('0' + random_value - 2 * 26); + } + } + return suffix; +} + } // namespace bool TouchFile(const base::FilePath& path, @@ -162,4 +184,133 @@ bool TouchFile(const base::FilePath& path) { return TouchFile(path, kPermissions600, geteuid(), getegid()); } +bool WriteBlobToFile(const base::FilePath& path, const Blob& blob) { + return WriteToFile(path, reinterpret_cast<const char*>(blob.data()), + blob.size()); +} + +bool WriteStringToFile(const base::FilePath& path, const std::string& data) { + return WriteToFile(path, data.data(), data.size()); +} + +bool WriteToFile(const base::FilePath& path, const char* data, size_t size) { + if (!base::DirectoryExists(path.DirName())) { + if (!base::CreateDirectory(path.DirName())) { + LOG(ERROR) << "Cannot create directory: " << path.DirName().value(); + return false; + } + } + // base::WriteFile takes an int size. + if (size > std::numeric_limits<int>::max()) { + LOG(ERROR) << "Cannot write to " << path.value() + << ". Data is too large: " << size << " bytes."; + return false; + } + + int data_written = base::WriteFile(path, data, size); + return data_written == static_cast<int>(size); +} + +bool SyncFileOrDirectory(const base::FilePath& path, + bool is_directory, + bool data_sync) { + const base::TimeTicks start = base::TimeTicks::Now(); + data_sync = data_sync && !is_directory; + + int flags = (is_directory ? O_RDONLY | O_DIRECTORY : O_WRONLY); + int fd = HANDLE_EINTR(open(path.value().c_str(), flags)); + if (fd < 0) { + PLOG(WARNING) << "Could not open " << path.value() << " for syncing"; + return false; + } + // POSIX specifies EINTR as a possible return value of fsync() but not for + // fdatasync(). To be on the safe side, it is handled in both cases. + int result = + (data_sync ? HANDLE_EINTR(fdatasync(fd)) : HANDLE_EINTR(fsync(fd))); + if (result < 0) { + PLOG(WARNING) << "Failed to sync " << path.value(); + close(fd); + return false; + } + // close() may not be retried on error. + result = IGNORE_EINTR(close(fd)); + if (result < 0) { + PLOG(WARNING) << "Failed to close after sync " << path.value(); + return false; + } + + const base::TimeDelta delta = base::TimeTicks::Now() - start; + if (delta > kLongSync) { + LOG(WARNING) << "Long " << (data_sync ? "fdatasync" : "fsync") << "() of " + << path.value() << ": " << delta.InSeconds() << " seconds"; + } + + return true; +} + +bool WriteToFileAtomic(const base::FilePath& path, + const char* data, + size_t size, + mode_t mode) { + if (!base::DirectoryExists(path.DirName())) { + if (!base::CreateDirectory(path.DirName())) { + LOG(ERROR) << "Cannot create directory: " << path.DirName().value(); + return false; + } + } + std::string random_suffix = GetRandomSuffix(); + if (random_suffix.empty()) { + PLOG(WARNING) << "Could not compute random suffix"; + return false; + } + std::string temp_name = path.AddExtension(random_suffix).value(); + int fd = + HANDLE_EINTR(open(temp_name.c_str(), O_CREAT | O_EXCL | O_WRONLY, mode)); + if (fd < 0) { + PLOG(WARNING) << "Could not open " << temp_name << " for atomic write"; + unlink(temp_name.c_str()); + return false; + } + + size_t position = 0; + while (position < size) { + ssize_t bytes_written = + HANDLE_EINTR(write(fd, data + position, size - position)); + if (bytes_written < 0) { + PLOG(WARNING) << "Could not write " << temp_name; + close(fd); + unlink(temp_name.c_str()); + return false; + } + position += bytes_written; + } + + if (HANDLE_EINTR(fdatasync(fd)) < 0) { + PLOG(WARNING) << "Could not fsync " << temp_name; + close(fd); + unlink(temp_name.c_str()); + return false; + } + if (close(fd) < 0) { + PLOG(WARNING) << "Could not close " << temp_name; + unlink(temp_name.c_str()); + return false; + } + + if (rename(temp_name.c_str(), path.value().c_str()) < 0) { + PLOG(WARNING) << "Could not close " << temp_name; + unlink(temp_name.c_str()); + return false; + } + + return true; +} + +bool WriteBlobToFileAtomic(const base::FilePath& path, + const Blob& blob, + mode_t mode) { + return WriteToFileAtomic(path, reinterpret_cast<const char*>(blob.data()), + blob.size(), mode); +} + } // namespace brillo diff --git a/brillo/file_utils.h b/brillo/file_utils.h index 60e0cdc..663d640 100644 --- a/brillo/file_utils.h +++ b/brillo/file_utils.h @@ -9,6 +9,7 @@ #include <base/files/file_path.h> #include <brillo/brillo_export.h> +#include <brillo/secure_blob.h> namespace brillo { @@ -30,6 +31,53 @@ BRILLO_EXPORT bool TouchFile(const base::FilePath& path, // bit set. BRILLO_EXPORT bool TouchFile(const base::FilePath& path); +// Writes the entirety of the given data to |path| with 0640 permissions +// (modulo umask). If missing, parent (and parent of parent etc.) directories +// are created with 0700 permissions (modulo umask). Returns true on success. +// +// Parameters +// path - Path of the file to write +// blob/data - blob/string/array to populate from +// (size - array size) +BRILLO_EXPORT bool WriteBlobToFile(const base::FilePath& path, + const Blob& blob); +BRILLO_EXPORT bool WriteStringToFile(const base::FilePath& path, + const std::string& data); +BRILLO_EXPORT bool WriteToFile(const base::FilePath& path, + const char* data, + size_t size); + +// Calls fdatasync() on file if data_sync is true or fsync() on directory or +// file when data_sync is false. Returns true on success. +// +// Parameters +// path - File/directory to be sync'ed +// is_directory - True if |path| is a directory +// data_sync - True if |path| does not need metadata to be synced +BRILLO_EXPORT bool SyncFileOrDirectory(const base::FilePath& path, + bool is_directory, + bool data_sync); + +// Atomically writes the entirety of the given data to |path| with |mode| +// permissions (modulo umask). If missing, parent (and parent of parent etc.) +// directories are created with 0700 permissions (modulo umask). Returns true +// if the file has been written successfully and it has physically hit the +// disk. Returns false if either writing the file has failed or if it cannot +// be guaranteed that it has hit the disk. +// +// Parameters +// path - Path of the file to write +// blob/data - blob/array to populate from +// (size - array size) +// mode - File permission bit-pattern, eg. 0644 for rw-r--r-- +BRILLO_EXPORT bool WriteBlobToFileAtomic(const base::FilePath& path, + const Blob& blob, + mode_t mode); +BRILLO_EXPORT bool WriteToFileAtomic(const base::FilePath& path, + const char* data, + size_t size, + mode_t mode); + } // namespace brillo #endif // LIBBRILLO_BRILLO_FILE_UTILS_H_ diff --git a/brillo/file_utils_unittest.cc b/brillo/file_utils_unittest.cc index e906db7..7a730f0 100644 --- a/brillo/file_utils_unittest.cc +++ b/brillo/file_utils_unittest.cc @@ -11,10 +11,28 @@ #include <base/files/file_util.h> #include <base/files/scoped_temp_dir.h> +#include <base/rand_util.h> +#include <base/strings/string_number_conversions.h> #include <gtest/gtest.h> namespace brillo { +namespace { + +constexpr int kPermissions600 = + base::FILE_PERMISSION_READ_BY_USER | base::FILE_PERMISSION_WRITE_BY_USER; +constexpr int kPermissions700 = base::FILE_PERMISSION_USER_MASK; +constexpr int kPermissions777 = base::FILE_PERMISSION_MASK; + +std::string GetRandomSuffix() { + const int kBufferSize = 6; + unsigned char buffer[kBufferSize]; + base::RandBytes(buffer, arraysize(buffer)); + return base::HexEncode(buffer, arraysize(buffer)); +} + +} // namespace + class FileUtilsTest : public testing::Test { public: FileUtilsTest() { @@ -47,19 +65,13 @@ class FileUtilsTest : public testing::Test { EXPECT_TRUE(base::GetPosixFilePermissions(file_path_, &actual_permissions)); EXPECT_EQ(permissions, actual_permissions); } -}; - -namespace { -enum { - kPermissions600 = - base::FILE_PERMISSION_READ_BY_USER | base::FILE_PERMISSION_WRITE_BY_USER, - kPermissions700 = base::FILE_PERMISSION_USER_MASK, - kPermissions777 = base::FILE_PERMISSION_MASK + // Creates a file with a random name in the temporary directory. + base::FilePath GetTempName() { + return temp_dir_.GetPath().Append(GetRandomSuffix()); + } }; -} // namespace - TEST_F(FileUtilsTest, TouchFileCreate) { EXPECT_TRUE(TouchFile(file_path_)); ExpectFileContains(""); @@ -132,4 +144,102 @@ TEST_F(FileUtilsTest, TouchFileExistingPermissionsUnchanged) { ExpectFilePermissions(kPermissions777); } +TEST_F(FileUtilsTest, WriteFileCanBeReadBack) { + const base::FilePath filename(GetTempName()); + const std::string content("blablabla"); + EXPECT_TRUE(WriteStringToFile(filename, content)); + std::string output; + EXPECT_TRUE(ReadFileToString(filename, &output)); + EXPECT_EQ(content, output); +} + +TEST_F(FileUtilsTest, WriteFileSets0666) { + const mode_t mask = 0000; + const mode_t mode = 0666; + const base::FilePath filename(GetTempName()); + const std::string content("blablabla"); + const mode_t old_mask = umask(mask); + EXPECT_TRUE(WriteStringToFile(filename, content)); + int file_mode = 0; + EXPECT_TRUE(base::GetPosixFilePermissions(filename, &file_mode)); + EXPECT_EQ(mode & ~mask, file_mode & 0777); + umask(old_mask); +} + +TEST_F(FileUtilsTest, WriteFileCreatesMissingParentDirectoriesWith0700) { + const mode_t mask = 0000; + const mode_t mode = 0700; + const base::FilePath dirname(GetTempName()); + const base::FilePath subdirname(dirname.Append(GetRandomSuffix())); + const base::FilePath filename(subdirname.Append(GetRandomSuffix())); + const std::string content("blablabla"); + EXPECT_TRUE(WriteStringToFile(filename, content)); + int dir_mode = 0; + int subdir_mode = 0; + EXPECT_TRUE(base::GetPosixFilePermissions(dirname, &dir_mode)); + EXPECT_TRUE(base::GetPosixFilePermissions(subdirname, &subdir_mode)); + EXPECT_EQ(mode & ~mask, dir_mode & 0777); + EXPECT_EQ(mode & ~mask, subdir_mode & 0777); + const mode_t old_mask = umask(mask); + umask(old_mask); +} + +TEST_F(FileUtilsTest, WriteToFileAtomicCanBeReadBack) { + const base::FilePath filename(GetTempName()); + const std::string content("blablabla"); + EXPECT_TRUE( + WriteToFileAtomic(filename, content.data(), content.size(), 0644)); + std::string output; + EXPECT_TRUE(ReadFileToString(filename, &output)); + EXPECT_EQ(content, output); +} + +TEST_F(FileUtilsTest, WriteToFileAtomicHonorsMode) { + const mode_t mask = 0000; + const mode_t mode = 0616; + const base::FilePath filename(GetTempName()); + const std::string content("blablabla"); + const mode_t old_mask = umask(mask); + EXPECT_TRUE( + WriteToFileAtomic(filename, content.data(), content.size(), mode)); + int file_mode = 0; + EXPECT_TRUE(base::GetPosixFilePermissions(filename, &file_mode)); + EXPECT_EQ(mode & ~mask, file_mode & 0777); + umask(old_mask); +} + +TEST_F(FileUtilsTest, WriteToFileAtomicHonorsUmask) { + const mode_t mask = 0073; + const mode_t mode = 0777; + const base::FilePath filename(GetTempName()); + const std::string content("blablabla"); + const mode_t old_mask = umask(mask); + EXPECT_TRUE( + WriteToFileAtomic(filename, content.data(), content.size(), mode)); + int file_mode = 0; + EXPECT_TRUE(base::GetPosixFilePermissions(filename, &file_mode)); + EXPECT_EQ(mode & ~mask, file_mode & 0777); + umask(old_mask); +} + +TEST_F(FileUtilsTest, + WriteToFileAtomicCreatesMissingParentDirectoriesWith0700) { + const mode_t mask = 0000; + const mode_t mode = 0700; + const base::FilePath dirname(GetTempName()); + const base::FilePath subdirname(dirname.Append(GetRandomSuffix())); + const base::FilePath filename(subdirname.Append(GetRandomSuffix())); + const std::string content("blablabla"); + EXPECT_TRUE( + WriteToFileAtomic(filename, content.data(), content.size(), 0777)); + int dir_mode = 0; + int subdir_mode = 0; + EXPECT_TRUE(base::GetPosixFilePermissions(dirname, &dir_mode)); + EXPECT_TRUE(base::GetPosixFilePermissions(subdirname, &subdir_mode)); + EXPECT_EQ(mode & ~mask, dir_mode & 0777); + EXPECT_EQ(mode & ~mask, subdir_mode & 0777); + const mode_t old_mask = umask(mask); + umask(old_mask); +} + } // namespace brillo diff --git a/brillo/glib/README.md b/brillo/glib/README.md new file mode 100644 index 0000000..ef9ec9e --- /dev/null +++ b/brillo/glib/README.md @@ -0,0 +1,5 @@ +# libbrillo GLib support + +GLib is deprecated in Chrome OS. Use [libchrome] instead. + +[libchrome]: ../../../libchrome diff --git a/brillo/glib/abstract_dbus_service.h b/brillo/glib/abstract_dbus_service.h index c40b658..32725c7 100644 --- a/brillo/glib/abstract_dbus_service.h +++ b/brillo/glib/abstract_dbus_service.h @@ -5,14 +5,14 @@ #ifndef LIBBRILLO_BRILLO_GLIB_ABSTRACT_DBUS_SERVICE_H_ #define LIBBRILLO_BRILLO_GLIB_ABSTRACT_DBUS_SERVICE_H_ +// IMPORTANT: Do not use this in new code. Instead, use +// <brillo/daemons/dbus_daemon.h>. See https://goo.gl/EH3MmR for more details. + #include <brillo/brillo_export.h> #include <brillo/glib/dbus.h> namespace brillo { -// \precondition No functions in the dbus namespace can be called before -// ::g_type_init(); - namespace dbus { class BRILLO_EXPORT AbstractDbusService { public: diff --git a/brillo/glib/dbus.h b/brillo/glib/dbus.h index 6006009..7a28480 100644 --- a/brillo/glib/dbus.h +++ b/brillo/glib/dbus.h @@ -5,6 +5,9 @@ #ifndef LIBBRILLO_BRILLO_GLIB_DBUS_H_ #define LIBBRILLO_BRILLO_GLIB_DBUS_H_ +// IMPORTANT: Do not use this in new code. Instead, use libchrome's D-Bus +// bindings. See https://goo.gl/EH3MmR for more details. + #include <dbus/dbus-glib.h> #include <glib-object.h> @@ -20,9 +23,6 @@ struct DBusConnection; namespace brillo { -// \precondition No functions in the dbus namespace can be called before -// ::g_type_init(); - namespace dbus { // \brief BusConnection manages the ref-count for a ::DBusGConnection*. diff --git a/brillo/glib/object.h b/brillo/glib/object.h index dfa96b6..15de52c 100644 --- a/brillo/glib/object.h +++ b/brillo/glib/object.h @@ -71,9 +71,6 @@ details::ResetHelper<T> Resetter(T* x) { return details::ResetHelper<T>(x); } -// \precondition No functions in the glib namespace can be called before -// ::g_type_init(); - namespace glib { // \brief type_to_gtypeid is a type function mapping from a canonical type to diff --git a/brillo/http/http_proxy.cc b/brillo/http/http_proxy.cc new file mode 100644 index 0000000..bf6a8af --- /dev/null +++ b/brillo/http/http_proxy.cc @@ -0,0 +1,133 @@ +// Copyright 2017 The Chromium OS 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 <brillo/http/http_proxy.h> + +#include <memory> +#include <string> +#include <vector> + +#include <base/bind.h> +#include <base/callback.h> +#include <base/logging.h> +#include <base/strings/string_tokenizer.h> +#include <base/strings/string_util.h> +#include <brillo/http/http_transport.h> +#include <chromeos/dbus/service_constants.h> +#include <dbus/bus.h> +#include <dbus/message.h> +#include <dbus/object_proxy.h> + +namespace { +bool ParseProxyInfo(dbus::Response* response, + std::vector<std::string>* proxies_out) { + DCHECK(proxies_out); + if (!response) { + LOG(ERROR) << chromeos::kNetworkProxyServiceName << " D-Bus call to " + << chromeos::kNetworkProxyServiceResolveProxyMethod + << " failed"; + proxies_out->assign({brillo::http::kDirectProxy}); + return false; + } + dbus::MessageReader reader(response); + std::string proxy_info; + std::string proxy_err; + if (!reader.PopString(&proxy_info) || !reader.PopString(&proxy_err)) { + LOG(ERROR) << chromeos::kNetworkProxyServiceName << " D-Bus call to " + << chromeos::kNetworkProxyServiceResolveProxyMethod + << " returned an invalid D-Bus response"; + proxies_out->assign({brillo::http::kDirectProxy}); + return false; + } + if (!proxy_err.empty()) { + // This case occurs when on the Chrome side of things it can't connect to + // the proxy resolver service, we just let this fall through and will end + // up returning success with only the direct proxy listed. + LOG(WARNING) << "Got error resolving proxy: " << proxy_err; + } + + base::StringTokenizer toker(proxy_info, ";"); + while (toker.GetNext()) { + std::string token = toker.token(); + base::TrimWhitespaceASCII(token, base::TRIM_ALL, &token); + + // Start by finding the first space (if any). + std::string::iterator space; + for (space = ++token.begin(); space != token.end(); ++space) { + if (base::IsAsciiWhitespace(*space)) { + break; + } + } + + std::string scheme = base::ToLowerASCII(std::string(token.begin(), space)); + // Chrome uses "socks" to mean socks4 and "proxy" to mean http. + if (scheme == "socks") { + scheme += "4"; + } else if (scheme == "proxy") { + scheme = "http"; + } else if (scheme != "https" && scheme != "socks4" && scheme != "socks5" && + scheme != "direct") { + LOG(ERROR) << "Invalid proxy scheme found of: " << scheme; + continue; + } + + std::string host_and_port = std::string(space, token.end()); + base::TrimWhitespaceASCII(host_and_port, base::TRIM_ALL, &host_and_port); + if (scheme != "direct" && host_and_port.empty()) { + LOG(ERROR) << "Invalid host/port information for proxy: " << token; + continue; + } + proxies_out->push_back(scheme + "://" + host_and_port); + } + // Always add the direct proxy (i.e. no proxy) as a last resort if not there. + if (proxies_out->empty() || + proxies_out->back() != brillo::http::kDirectProxy) { + proxies_out->push_back(brillo::http::kDirectProxy); + } + return true; +} + +void OnResolveProxy(const brillo::http::GetChromeProxyServersCallback& callback, + dbus::Response* response) { + std::vector<std::string> proxies; + bool result = ParseProxyInfo(response, &proxies); + callback.Run(result, std::move(proxies)); +} +} // namespace + +namespace brillo { +namespace http { + +bool GetChromeProxyServers(scoped_refptr<dbus::Bus> bus, const std::string& url, + std::vector<std::string>* proxies_out) { + dbus::ObjectProxy* proxy = + bus->GetObjectProxy(chromeos::kNetworkProxyServiceName, + dbus::ObjectPath(chromeos::kNetworkProxyServicePath)); + dbus::MethodCall method_call( + chromeos::kNetworkProxyServiceInterface, + chromeos::kNetworkProxyServiceResolveProxyMethod); + dbus::MessageWriter writer(&method_call); + writer.AppendString(url); + std::unique_ptr<dbus::Response> response = proxy->CallMethodAndBlock( + &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT); + return ParseProxyInfo(response.get(), proxies_out); +} + +void GetChromeProxyServersAsync(scoped_refptr<dbus::Bus> bus, + const std::string& url, + const GetChromeProxyServersCallback& callback) { + dbus::ObjectProxy* proxy = bus->GetObjectProxy( + chromeos::kNetworkProxyServiceName, + dbus::ObjectPath(chromeos::kNetworkProxyServicePath)); + dbus::MethodCall method_call( + chromeos::kNetworkProxyServiceInterface, + chromeos::kNetworkProxyServiceResolveProxyMethod); + dbus::MessageWriter writer(&method_call); + writer.AppendString(url); + proxy->CallMethod(&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, + base::Bind(&OnResolveProxy, callback)); +} + +} // namespace http +} // namespace brillo diff --git a/brillo/http/http_proxy.h b/brillo/http/http_proxy.h new file mode 100644 index 0000000..c142af2 --- /dev/null +++ b/brillo/http/http_proxy.h @@ -0,0 +1,48 @@ +// Copyright 2017 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef LIBBRILLO_BRILLO_HTTP_HTTP_PROXY_H_ +#define LIBBRILLO_BRILLO_HTTP_HTTP_PROXY_H_ + +#include <string> +#include <vector> + +#include <base/callback_forward.h> +#include <base/memory/ref_counted.h> +#include <brillo/brillo_export.h> + +namespace dbus { +class Bus; +} // namespace dbus + +namespace brillo { +namespace http { + +using GetChromeProxyServersCallback = + base::Callback<void(bool success, const std::vector<std::string>& proxies)>; + +// Gets the list of proxy servers that are configured in Chrome by sending a +// D-Bus message to Chrome. The list will always be at least one in size, with +// the last element always being the direct option. The target URL should be +// passed in for the |url| parameter. The proxy servers are set in +// |proxies_out|. The format of the strings in |proxies_out| is +// scheme://[[user:pass@]host:port] with the last element being "direct://". +// Valid schemes it will return are socks4, socks5, http, https or direct. +// Even if this function returns false, it will still set |proxies_out| to be +// just the direct proxy. This function will only return false if there is an +// error in the D-Bus communication itself. +BRILLO_EXPORT bool GetChromeProxyServers(scoped_refptr<dbus::Bus> bus, + const std::string& url, + std::vector<std::string>* proxies_out); + +// Async version of GetChromeProxyServers. +BRILLO_EXPORT void GetChromeProxyServersAsync( + scoped_refptr<dbus::Bus> bus, + const std::string& url, + const GetChromeProxyServersCallback& callback); + +} // namespace http +} // namespace brillo + +#endif // LIBBRILLO_BRILLO_HTTP_HTTP_PROXY_H_ diff --git a/brillo/http/http_proxy_unittest.cc b/brillo/http/http_proxy_unittest.cc new file mode 100644 index 0000000..4893a87 --- /dev/null +++ b/brillo/http/http_proxy_unittest.cc @@ -0,0 +1,180 @@ +// Copyright 2017 The Chromium OS 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 <brillo/http/http_proxy.h> + +#include <string> +#include <vector> + +#include <base/bind.h> +#include <brillo/http/http_transport.h> +#include <chromeos/dbus/service_constants.h> +#include <dbus/mock_bus.h> +#include <dbus/mock_object_proxy.h> +#include <gtest/gtest.h> + +using ::testing::_; +using ::testing::ElementsAre; +using ::testing::Invoke; +using ::testing::Return; + +namespace { +constexpr char kTestUrl[] = "http://www.example.com/test"; +} // namespace + +namespace brillo { +namespace http { + +class HttpProxyTest : public testing::Test { + public: + void ResolveProxyHandlerAsync(dbus::MethodCall* method_call, + int timeout_msec, + dbus::ObjectProxy::ResponseCallback callback) { + if (null_dbus_response_) { + callback.Run(nullptr); + return; + } + callback.Run(CreateDBusResponse(method_call).get()); + } + + dbus::Response* ResolveProxyHandler(dbus::MethodCall* method_call, + int timeout_msec) { + if (null_dbus_response_) { + return nullptr; + } + // The mock wraps this back into a std::unique_ptr in the function calling + // us. + return CreateDBusResponse(method_call).release(); + } + + MOCK_METHOD2(GetProxiesCallback, void(bool, const std::vector<std::string>&)); + + protected: + HttpProxyTest() { + dbus::Bus::Options options; + options.bus_type = dbus::Bus::SYSTEM; + bus_ = new dbus::MockBus(options); + object_proxy_ = new dbus::MockObjectProxy( + bus_.get(), chromeos::kNetworkProxyServiceName, + dbus::ObjectPath(chromeos::kNetworkProxyServicePath)); + EXPECT_CALL(*bus_, GetObjectProxy(chromeos::kNetworkProxyServiceName, + dbus::ObjectPath( + chromeos::kNetworkProxyServicePath))) + .WillOnce(Return(object_proxy_.get())); + } + + std::unique_ptr<dbus::Response> CreateDBusResponse( + dbus::MethodCall* method_call) { + EXPECT_EQ(method_call->GetInterface(), + chromeos::kNetworkProxyServiceInterface); + EXPECT_EQ(method_call->GetMember(), + chromeos::kNetworkProxyServiceResolveProxyMethod); + method_call->SetSerial(1); // Needs to be non-zero or it fails. + std::unique_ptr<dbus::Response> response = + dbus::Response::FromMethodCall(method_call); + dbus::MessageWriter writer(response.get()); + writer.AppendString(proxy_info_); + if (invalid_dbus_response_) { + return response; + } + writer.AppendString(proxy_err_); + return response; + } + + scoped_refptr<dbus::MockBus> bus_; + scoped_refptr<dbus::MockObjectProxy> object_proxy_; + + std::string proxy_info_; + std::string proxy_err_; + bool null_dbus_response_ = false; + bool invalid_dbus_response_ = false; + + private: + DISALLOW_COPY_AND_ASSIGN(HttpProxyTest); +}; + +TEST_F(HttpProxyTest, DBusNullResponseFails) { + std::vector<std::string> proxies; + null_dbus_response_ = true; + EXPECT_CALL(*object_proxy_, MockCallMethodAndBlock(_, _)) + .WillOnce(Invoke(this, &HttpProxyTest::ResolveProxyHandler)); + EXPECT_FALSE(GetChromeProxyServers(bus_, kTestUrl, &proxies)); +} + +TEST_F(HttpProxyTest, DBusInvalidResponseFails) { + std::vector<std::string> proxies; + invalid_dbus_response_ = true; + EXPECT_CALL(*object_proxy_, MockCallMethodAndBlock(_, _)) + .WillOnce(Invoke(this, &HttpProxyTest::ResolveProxyHandler)); + EXPECT_FALSE(GetChromeProxyServers(bus_, kTestUrl, &proxies)); +} + +TEST_F(HttpProxyTest, NoProxies) { + std::vector<std::string> proxies; + EXPECT_CALL(*object_proxy_, MockCallMethodAndBlock(_, _)) + .WillOnce(Invoke(this, &HttpProxyTest::ResolveProxyHandler)); + EXPECT_TRUE(GetChromeProxyServers(bus_, kTestUrl, &proxies)); + EXPECT_THAT(proxies, ElementsAre(kDirectProxy)); +} + +TEST_F(HttpProxyTest, MultipleProxiesWithoutDirect) { + proxy_info_ = "proxy example.com; socks5 foo.com;"; + std::vector<std::string> proxies; + EXPECT_CALL(*object_proxy_, MockCallMethodAndBlock(_, _)) + .WillOnce(Invoke(this, &HttpProxyTest::ResolveProxyHandler)); + EXPECT_TRUE(GetChromeProxyServers(bus_, kTestUrl, &proxies)); + EXPECT_THAT(proxies, ElementsAre("http://example.com", "socks5://foo.com", + kDirectProxy)); +} + +TEST_F(HttpProxyTest, MultipleProxiesWithDirect) { + proxy_info_ = "socks foo.com; Https example.com ; badproxy example2.com ; " + "socks5 test.com ; proxy foobar.com; DIRECT "; + std::vector<std::string> proxies; + EXPECT_CALL(*object_proxy_, MockCallMethodAndBlock(_, _)) + .WillOnce(Invoke(this, &HttpProxyTest::ResolveProxyHandler)); + EXPECT_TRUE(GetChromeProxyServers(bus_, kTestUrl, &proxies)); + EXPECT_THAT(proxies, ElementsAre("socks4://foo.com", "https://example.com", + "socks5://test.com", "http://foobar.com", + kDirectProxy)); +} + +TEST_F(HttpProxyTest, DBusNullResponseFailsAsync) { + null_dbus_response_ = true; + EXPECT_CALL(*object_proxy_, CallMethod(_, _, _)) + .WillOnce(Invoke(this, &HttpProxyTest::ResolveProxyHandlerAsync)); + EXPECT_CALL(*this, GetProxiesCallback(false, _)); + GetChromeProxyServersAsync( + bus_, kTestUrl, + base::Bind(&HttpProxyTest::GetProxiesCallback, base::Unretained(this))); +} + +TEST_F(HttpProxyTest, DBusInvalidResponseFailsAsync) { + invalid_dbus_response_ = true; + EXPECT_CALL(*object_proxy_, CallMethod(_, _, _)) + .WillOnce(Invoke(this, &HttpProxyTest::ResolveProxyHandlerAsync)); + EXPECT_CALL(*this, GetProxiesCallback(false, _)); + GetChromeProxyServersAsync( + bus_, kTestUrl, + base::Bind(&HttpProxyTest::GetProxiesCallback, base::Unretained(this))); +} + +// We don't need to test all the proxy cases with async because that will be +// using the same internal parsing code. +TEST_F(HttpProxyTest, MultipleProxiesWithDirectAsync) { + proxy_info_ = "socks foo.com; Https example.com ; badproxy example2.com ; " + "socks5 test.com ; proxy foobar.com; DIRECT "; + std::vector<std::string> expected = { + "socks4://foo.com", "https://example.com", "socks5://test.com", + "http://foobar.com", kDirectProxy}; + EXPECT_CALL(*object_proxy_, CallMethod(_, _, _)) + .WillOnce(Invoke(this, &HttpProxyTest::ResolveProxyHandlerAsync)); + EXPECT_CALL(*this, GetProxiesCallback(true, expected)); + GetChromeProxyServersAsync( + bus_, kTestUrl, + base::Bind(&HttpProxyTest::GetProxiesCallback, base::Unretained(this))); +} + +} // namespace http +} // namespace brillo diff --git a/brillo/http/http_request_unittest.cc b/brillo/http/http_request_unittest.cc index 387c92d..39ccc18 100644 --- a/brillo/http/http_request_unittest.cc +++ b/brillo/http/http_request_unittest.cc @@ -156,26 +156,24 @@ TEST_F(HttpRequestTest, GetResponse) { return true; }; - auto success_callback = [](decltype(this) test, - const std::string& resp_data, - RequestID request_id, - std::unique_ptr<Response> resp) { + auto success_callback = base::Bind( + [](MockConnection* connection, const std::string& resp_data, + RequestID request_id, std::unique_ptr<Response> resp) { EXPECT_EQ(23, request_id); - EXPECT_CALL(*test->connection_, GetResponseStatusCode()) + EXPECT_CALL(*connection, GetResponseStatusCode()) .WillOnce(Return(status_code::Partial)); EXPECT_EQ(status_code::Partial, resp->GetStatusCode()); - EXPECT_CALL(*test->connection_, GetResponseStatusText()) + EXPECT_CALL(*connection, GetResponseStatusText()) .WillOnce(Return("Partial completion")); EXPECT_EQ("Partial completion", resp->GetStatusText()); - EXPECT_CALL(*test->connection_, - GetResponseHeader(response_header::kContentType)) + EXPECT_CALL(*connection, GetResponseHeader(response_header::kContentType)) .WillOnce(Return(mime::text::kHtml)); EXPECT_EQ(mime::text::kHtml, resp->GetContentType()); EXPECT_EQ(resp_data, resp->ExtractDataAsString()); - }; + }, connection_.get(), resp_data); auto finish_request_async = [this, &read_data](const SuccessCallback& success_callback) { @@ -198,10 +196,7 @@ TEST_F(HttpRequestTest, GetResponse) { EXPECT_CALL(*connection_, FinishRequestAsync(_, _)) .WillOnce(DoAll(WithArg<0>(Invoke(finish_request_async)), Return(23))); - EXPECT_EQ( - 23, - request.GetResponse( - base::Bind(success_callback, base::Unretained(this), resp_data), {})); + EXPECT_EQ(23, request.GetResponse(success_callback, {})); } } // namespace http diff --git a/brillo/http/http_transport.cc b/brillo/http/http_transport.cc index d77eabe..0c27489 100644 --- a/brillo/http/http_transport.cc +++ b/brillo/http/http_transport.cc @@ -10,10 +10,21 @@ namespace brillo { namespace http { const char kErrorDomain[] = "http_transport"; +const char kDirectProxy[] = "direct://"; std::shared_ptr<Transport> Transport::CreateDefault() { return std::make_shared<http::curl::Transport>(std::make_shared<CurlApi>()); } +std::shared_ptr<Transport> Transport::CreateDefaultWithProxy( + const std::string& proxy) { + if (proxy.empty() || proxy == kDirectProxy) { + return CreateDefault(); + } else { + return std::make_shared<http::curl::Transport>(std::make_shared<CurlApi>(), + proxy); + } +} + } // namespace http } // namespace brillo diff --git a/brillo/http/http_transport.h b/brillo/http/http_transport.h index 63e135c..768859e 100644 --- a/brillo/http/http_transport.h +++ b/brillo/http/http_transport.h @@ -21,6 +21,8 @@ namespace brillo { namespace http { BRILLO_EXPORT extern const char kErrorDomain[]; +// Constant referring to 'direct' proxy which implies no proxy server. +BRILLO_EXPORT extern const char kDirectProxy[]; // direct:// class Request; class Response; @@ -82,9 +84,19 @@ class BRILLO_EXPORT Transport : public std::enable_shared_from_this<Transport> { // Set the default timeout of requests made. virtual void SetDefaultTimeout(base::TimeDelta timeout) = 0; + // Set the local IP address of requests + virtual void SetLocalIpAddress(const std::string& ip_address) = 0; + // Creates a default http::Transport (currently, using http::curl::Transport). static std::shared_ptr<Transport> CreateDefault(); + // Creates a default http::Transport that will utilize the passed in proxy + // server (currently, using a http::curl::Transport). |proxy| should be of the + // form scheme://[user:pass@]host:port or may be the empty string or the + // string kDirectProxy (i.e. direct://) to indicate no proxy. + static std::shared_ptr<Transport> CreateDefaultWithProxy( + const std::string& proxy); + private: DISALLOW_COPY_AND_ASSIGN(Transport); }; diff --git a/brillo/http/http_transport_curl.cc b/brillo/http/http_transport_curl.cc index 109d2c1..e0d78d5 100644 --- a/brillo/http/http_transport_curl.cc +++ b/brillo/http/http_transport_curl.cc @@ -130,7 +130,7 @@ std::shared_ptr<http::Connection> Transport::CreateConnection( return connection; } - LOG(INFO) << "Sending a " << method << " request to " << url; + VLOG(1) << "Sending a " << method << " request to " << url; CURLcode code = curl_interface_->EasySetOptStr(curl_handle, CURLOPT_URL, url); if (code == CURLE_OK) { @@ -165,6 +165,10 @@ std::shared_ptr<http::Connection> Transport::CreateConnection( static_cast<int>(timeout_ms)); } } + if (code == CURLE_OK && !ip_address_.empty()) { + code = curl_interface_->EasySetOptStr( + curl_handle, CURLOPT_INTERFACE, ip_address_.c_str()); + } // Setup HTTP request method and optional request body. if (code == CURLE_OK) { @@ -245,7 +249,7 @@ RequestID Transport::StartAsyncTransfer(http::Connection* connection, request_id_map_.erase(request_id); return 0; } - LOG(INFO) << "Started asynchronous HTTP request with ID " << request_id; + VLOG(1) << "Started asynchronous HTTP request with ID " << request_id; return request_id; } @@ -266,6 +270,10 @@ void Transport::SetDefaultTimeout(base::TimeDelta timeout) { connection_timeout_ = timeout; } +void Transport::SetLocalIpAddress(const std::string& ip_address) { + ip_address_ = "host!" + ip_address; +} + void Transport::AddEasyCurlError(brillo::ErrorPtr* error, const tracked_objects::Location& location, CURLcode code, @@ -354,8 +362,8 @@ int Transport::MultiSocketCallback(CURL* easy, poll_data->GetWatcher()->StopWatchingFileDescriptor(); // This method can be called indirectly from SocketPollData::OnSocketReady, // so delay destruction of SocketPollData object till the next loop cycle. - base::MessageLoopForIO::current()->task_runner()-> - DeleteSoon(FROM_HERE, poll_data); + base::MessageLoopForIO::current()->task_runner()->DeleteSoon(FROM_HERE, + poll_data); return 0; } @@ -439,9 +447,9 @@ void Transport::OnTransferComplete(Connection* connection, CURLcode code) { auto p = async_requests_.find(connection); CHECK(p != async_requests_.end()) << "Unknown connection"; AsyncRequestData* request_data = p->second.get(); - LOG(INFO) << "HTTP request # " << request_data->request_id - << " has completed " - << (code == CURLE_OK ? "successfully" : "with an error"); + VLOG(1) << "HTTP request # " << request_data->request_id + << " has completed " + << (code == CURLE_OK ? "successfully" : "with an error"); if (code != CURLE_OK) { brillo::ErrorPtr error; AddEasyCurlError(&error, FROM_HERE, code, curl_interface_.get()); @@ -450,8 +458,10 @@ void Transport::OnTransferComplete(Connection* connection, CURLcode code) { p->second->request_id, base::Owned(error.release()))); } else { - LOG(INFO) << "Response: " << connection->GetResponseStatusCode() << " (" - << connection->GetResponseStatusText() << ")"; + if (connection->GetResponseStatusCode() != status_code::Ok) { + LOG(INFO) << "Response: " << connection->GetResponseStatusCode() << " (" + << connection->GetResponseStatusText() << ")"; + } brillo::ErrorPtr error; // Rewind the response data stream to the beginning so the clients can // read the data back. diff --git a/brillo/http/http_transport_curl.h b/brillo/http/http_transport_curl.h index 0a95c63..518ee9d 100644 --- a/brillo/http/http_transport_curl.h +++ b/brillo/http/http_transport_curl.h @@ -59,6 +59,8 @@ class BRILLO_EXPORT Transport : public http::Transport { void SetDefaultTimeout(base::TimeDelta timeout) override; + void SetLocalIpAddress(const std::string& ip_address) override; + // Helper methods to convert CURL error codes (CURLcode and CURLMcode) // into brillo::Error object. static void AddEasyCurlError(brillo::ErrorPtr* error, @@ -127,6 +129,7 @@ class BRILLO_EXPORT Transport : public http::Transport { RequestID last_request_id_{0}; // The connection timeout for the requests made. base::TimeDelta connection_timeout_; + std::string ip_address_; base::WeakPtrFactory<Transport> weak_ptr_factory_for_timer_{this}; base::WeakPtrFactory<Transport> weak_ptr_factory_{this}; diff --git a/brillo/http/http_transport_curl_unittest.cc b/brillo/http/http_transport_curl_unittest.cc index 1937e73..c05c81a 100644 --- a/brillo/http/http_transport_curl_unittest.cc +++ b/brillo/http/http_transport_curl_unittest.cc @@ -78,6 +78,36 @@ TEST_F(HttpCurlTransportTest, RequestGet) { connection.reset(); } +TEST_F(HttpCurlTransportTest, RequestGetWithProxy) { + EXPECT_CALL(*curl_api_, + EasySetOptStr(handle_, CURLOPT_URL, "http://foo.bar/get")) + .WillOnce(Return(CURLE_OK)); + EXPECT_CALL(*curl_api_, + EasySetOptStr(handle_, CURLOPT_USERAGENT, "User Agent")) + .WillOnce(Return(CURLE_OK)); + EXPECT_CALL(*curl_api_, + EasySetOptStr(handle_, CURLOPT_REFERER, "http://foo.bar/baz")) + .WillOnce(Return(CURLE_OK)); + EXPECT_CALL(*curl_api_, + EasySetOptStr(handle_, CURLOPT_PROXY, "http://proxy.server")) + .WillOnce(Return(CURLE_OK)); + EXPECT_CALL(*curl_api_, EasySetOptInt(handle_, CURLOPT_HTTPGET, 1)) + .WillOnce(Return(CURLE_OK)); + std::shared_ptr<Transport> proxy_transport = + std::make_shared<Transport>(curl_api_, "http://proxy.server"); + + auto connection = proxy_transport->CreateConnection("http://foo.bar/get", + request_type::kGet, + {}, + "User Agent", + "http://foo.bar/baz", + nullptr); + EXPECT_NE(nullptr, connection.get()); + + EXPECT_CALL(*curl_api_, EasyCleanup(handle_)).Times(1); + connection.reset(); +} + TEST_F(HttpCurlTransportTest, RequestHead) { EXPECT_CALL(*curl_api_, EasySetOptStr(handle_, CURLOPT_URL, "http://foo.bar/head")) @@ -205,14 +235,13 @@ TEST_F(HttpCurlTransportAsyncTest, StartAsyncTransfer) { // Success/error callback needed to report the result of an async operation. int success_call_count = 0; - auto success_callback = [](int* success_call_count, - base::RunLoop* run_loop, - RequestID /* request_id */, - std::unique_ptr<http::Response> /* resp */) { + auto success_callback = base::Bind([]( + int* success_call_count, const base::Closure& quit_closure, + RequestID /* request_id */, std::unique_ptr<http::Response> /* resp */) { base::MessageLoop::current()->task_runner()->PostTask( - FROM_HERE, run_loop->QuitClosure()); + FROM_HERE, quit_closure); (*success_call_count)++; - }; + }, &success_call_count, run_loop.QuitClosure()); auto error_callback = [](RequestID /* request_id */, const Error* /* error */) { @@ -236,13 +265,9 @@ TEST_F(HttpCurlTransportAsyncTest, StartAsyncTransfer) { EXPECT_CALL(*curl_api_, MultiAddHandle(multi_handle_, handle_)) .WillOnce(Return(CURLM_OK)); - EXPECT_EQ(1, - transport_->StartAsyncTransfer( - connection.get(), - base::Bind(success_callback, - base::Unretained(&success_call_count), - base::Unretained(&run_loop)), - base::Bind(error_callback))); + EXPECT_EQ(1, transport_->StartAsyncTransfer(connection.get(), + success_callback, + base::Bind(error_callback))); EXPECT_EQ(0, success_call_count); timer_callback(multi_handle_, 1, transport_.get()); diff --git a/brillo/http/http_transport_fake.h b/brillo/http/http_transport_fake.h index ac8bc2a..e894c32 100644 --- a/brillo/http/http_transport_fake.h +++ b/brillo/http/http_transport_fake.h @@ -102,6 +102,8 @@ class Transport : public http::Transport { void SetDefaultTimeout(base::TimeDelta timeout) override; + void SetLocalIpAddress(const std::string& ip_address) override {} + private: // A list of user-supplied request handlers. std::map<std::string, HandlerCallback> handlers_; diff --git a/brillo/http/mock_transport.h b/brillo/http/mock_transport.h index 9bc6c14..040d20e 100644 --- a/brillo/http/mock_transport.h +++ b/brillo/http/mock_transport.h @@ -33,6 +33,7 @@ class MockTransport : public Transport { const ErrorCallback&)); MOCK_METHOD1(CancelRequest, bool(RequestID)); MOCK_METHOD1(SetDefaultTimeout, void(base::TimeDelta)); + MOCK_METHOD1(SetLocalIpAddress, void(const std::string&)); private: DISALLOW_COPY_AND_ASSIGN(MockTransport); diff --git a/brillo/imageloader/manifest.cc b/brillo/imageloader/manifest.cc new file mode 100644 index 0000000..92789df --- /dev/null +++ b/brillo/imageloader/manifest.cc @@ -0,0 +1,188 @@ +// Copyright 2018 The Chromium OS 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 <brillo/imageloader/manifest.h> + +#include <memory> +#include <utility> + +#include <base/json/json_string_value_serializer.h> +#include <base/strings/string_number_conversions.h> + +namespace brillo { +namespace imageloader { + +namespace { +// The current version of the manifest file. +constexpr int kCurrentManifestVersion = 1; +// The name of the version field in the manifest. +constexpr char kManifestVersionField[] = "manifest-version"; +// The name of the component version field in the manifest. +constexpr char kVersionField[] = "version"; +// The name of the field containing the image hash. +constexpr char kImageHashField[] = "image-sha256-hash"; +// The name of the bool field indicating whether component is removable. +constexpr char kIsRemovableField[] = "is-removable"; +// The name of the metadata field. +constexpr char kMetadataField[] = "metadata"; +// The name of the field containing the table hash. +constexpr char kTableHashField[] = "table-sha256-hash"; +// The name of the optional field containing the file system type. +constexpr char kFSType[] = "fs-type"; + +bool GetSHA256FromString(const std::string& hash_str, + std::vector<uint8_t>* bytes) { + if (!base::HexStringToBytes(hash_str, bytes)) + return false; + return bytes->size() == 32; +} + +// Ensure the metadata entry is a dictionary mapping strings to strings and +// parse it into |out_metadata| and return true if so. +bool ParseMetadata(const base::Value* metadata_element, + std::map<std::string, std::string>* out_metadata) { + DCHECK(out_metadata); + + const base::DictionaryValue* metadata_dict = nullptr; + if (!metadata_element->GetAsDictionary(&metadata_dict)) + return false; + + base::DictionaryValue::Iterator it(*metadata_dict); + for (; !it.IsAtEnd(); it.Advance()) { + std::string parsed_value; + if (!it.value().GetAsString(&parsed_value)) { + LOG(ERROR) << "Key \"" << it.key() << "\" did not map to string value"; + return false; + } + + (*out_metadata)[it.key()] = std::move(parsed_value); + } + + return true; +} + +} // namespace + +Manifest::Manifest() {} + +bool Manifest::ParseManifest(const std::string& manifest_raw) { + // Now deserialize the manifest json and read out the rest of the component. + int error_code; + std::string error_message; + JSONStringValueDeserializer deserializer(manifest_raw); + std::unique_ptr<base::Value> value = + deserializer.Deserialize(&error_code, &error_message); + + if (!value) { + LOG(ERROR) << "Could not deserialize the manifest file. Error " + << error_code << ": " << error_message; + return false; + } + + base::DictionaryValue* manifest_dict = nullptr; + if (!value->GetAsDictionary(&manifest_dict)) { + LOG(ERROR) << "Could not parse manifest file as JSON."; + return false; + } + + // This will have to be changed if the manifest version is bumped. + int version; + if (!manifest_dict->GetInteger(kManifestVersionField, &version)) { + LOG(ERROR) << "Could not parse manifest version field from manifest."; + return false; + } + if (version != kCurrentManifestVersion) { + LOG(ERROR) << "Unsupported version of the manifest."; + return false; + } + manifest_version_ = version; + + std::string image_hash_str; + if (!manifest_dict->GetString(kImageHashField, &image_hash_str)) { + LOG(ERROR) << "Could not parse image hash from manifest."; + return false; + } + + if (!GetSHA256FromString(image_hash_str, &(image_sha256_))) { + LOG(ERROR) << "Could not convert image hash to bytes."; + return false; + } + + std::string table_hash_str; + if (!manifest_dict->GetString(kTableHashField, &table_hash_str)) { + LOG(ERROR) << "Could not parse table hash from manifest."; + return false; + } + + if (!GetSHA256FromString(table_hash_str, &(table_sha256_))) { + LOG(ERROR) << "Could not convert table hash to bytes."; + return false; + } + + if (!manifest_dict->GetString(kVersionField, &(version_))) { + LOG(ERROR) << "Could not parse component version from manifest."; + return false; + } + + // The fs_type field is optional, and squashfs by default. + fs_type_ = FileSystem::kSquashFS; + std::string fs_type; + if (manifest_dict->GetString(kFSType, &fs_type)) { + if (fs_type == "ext4") { + fs_type_ = FileSystem::kExt4; + } else if (fs_type == "squashfs") { + fs_type_ = FileSystem::kSquashFS; + } else { + LOG(ERROR) << "Unsupported file system type: " << fs_type; + return false; + } + } + + if (!manifest_dict->GetBoolean(kIsRemovableField, &(is_removable_))) { + // If is_removable field does not exist, by default it is false. + is_removable_ = false; + } + + // Copy out the metadata, if it's there. + const base::Value* metadata = nullptr; + if (manifest_dict->Get(kMetadataField, &metadata)) { + if (!ParseMetadata(metadata, &(metadata_))) { + LOG(ERROR) << "Manifest metadata was malformed"; + return false; + } + } + + return true; +} + +int Manifest::manifest_version() const { + return manifest_version_; +} + +const std::vector<uint8_t>& Manifest::image_sha256() const { + return image_sha256_; +} + +const std::vector<uint8_t>& Manifest::table_sha256() const { + return table_sha256_; +} + +const std::string& Manifest::version() const { + return version_; +} + +FileSystem Manifest::fs_type() const { + return fs_type_; +} + +bool Manifest::is_removable() const { + return is_removable_; +} + +const std::map<std::string, std::string> Manifest::metadata() const { + return metadata_; +} + +} // namespace imageloader +} // namespace brillo diff --git a/brillo/imageloader/manifest.h b/brillo/imageloader/manifest.h new file mode 100644 index 0000000..cfd7c3a --- /dev/null +++ b/brillo/imageloader/manifest.h @@ -0,0 +1,52 @@ +// Copyright 2018 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef LIBBRILLO_BRILLO_IMAGELOADER_MANIFEST_H_ +#define LIBBRILLO_BRILLO_IMAGELOADER_MANIFEST_H_ + +#include <map> +#include <string> +#include <vector> + +#include <base/macros.h> +#include <brillo/brillo_export.h> + +namespace brillo { +namespace imageloader { + +// The supported file systems for images. +enum class FileSystem { kExt4, kSquashFS }; + +// A class to parse and store imageloader.json manifest. +class BRILLO_EXPORT Manifest { + public: + Manifest(); + // Parse the manifest raw string. Return true if successful. + bool ParseManifest(const std::string& manifest_raw); + // Getters for manifest fields: + int manifest_version() const; + const std::vector<uint8_t>& image_sha256() const; + const std::vector<uint8_t>& table_sha256() const; + const std::string& version() const; + FileSystem fs_type() const; + bool is_removable() const; + const std::map<std::string, std::string> metadata() const; + + private: + // Manifest fields: + int manifest_version_; + std::vector<uint8_t> image_sha256_; + std::vector<uint8_t> table_sha256_; + std::string version_; + FileSystem fs_type_; + bool is_removable_; + std::map<std::string, std::string> metadata_; + + DISALLOW_COPY_AND_ASSIGN(Manifest); +}; + +} // namespace imageloader +} // namespace brillo + +#endif // LIBBRILLO_BRILLO_IMAGELOADER_MANIFEST_H_ diff --git a/brillo/imageloader/manifest_unittest.cc b/brillo/imageloader/manifest_unittest.cc new file mode 100644 index 0000000..bca7e8b --- /dev/null +++ b/brillo/imageloader/manifest_unittest.cc @@ -0,0 +1,49 @@ +// Copyright 2018 The Chromium OS 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 <gtest/gtest.h> + +#include <brillo/imageloader/manifest.h> + +namespace brillo { +namespace imageloader { + +class ManifestTest : public testing::Test {}; + +TEST_F(ManifestTest, ParseManifest) { + const std::string fs_type = R"("ext4")"; + const std::string is_removable = R"(true)"; + const std::string image_sha256_hash = + R"("4CF41BD11362CCB4707FB93939DBB5AC48745EDFC9DC8D7702852FFAA81B3B3F")"; + const std::string table_sha256_hash = + R"("0E11DA3D7140C6B95496787F50D15152434EBA22B60443BFA7E054FF4C799276")"; + const std::string version = R"("9824.0.4")"; + const std::string manifest_version = R"(1)"; + const std::string manifest_raw = std::string() + R"( + { + "fs-type":)" + fs_type + R"(, + "is-removable":)" + is_removable + + R"(, + "image-sha256-hash":)" + image_sha256_hash + + R"(, + "table-sha256-hash":)" + table_sha256_hash + + R"(, + "version":)" + version + R"(, + "manifest-version":)" + manifest_version + + R"( + } + )"; + brillo::imageloader::Manifest manifest; + // Parse the manifest raw string. + ASSERT_TRUE(manifest.ParseManifest(manifest_raw)); + EXPECT_EQ(manifest.fs_type(), FileSystem::kExt4); + EXPECT_EQ(manifest.is_removable(), true); + EXPECT_NE(manifest.image_sha256().size(), 0); + EXPECT_NE(manifest.table_sha256().size(), 0); + EXPECT_NE(manifest.version().size(), 0); + EXPECT_EQ(manifest.manifest_version(), 1); +} + +} // namespace imageloader +} // namespace brillo diff --git a/brillo/key_value_store_unittest.cc b/brillo/key_value_store_unittest.cc index 04af5fd..68875ef 100644 --- a/brillo/key_value_store_unittest.cc +++ b/brillo/key_value_store_unittest.cc @@ -41,7 +41,8 @@ TEST_F(KeyValueStoreTest, LoadAndSaveFromFile) { base::ScopedTempDir temp_dir_; CHECK(temp_dir_.CreateUniqueTempDir()); base::FilePath temp_file_ = temp_dir_.GetPath().Append("temp.conf"); - base::FilePath saved_temp_file_ = temp_dir_.GetPath().Append("saved_temp.conf"); + base::FilePath saved_temp_file_ = + temp_dir_.GetPath().Append("saved_temp.conf"); string blob = "A=B\n# Comment\n"; ASSERT_EQ(blob.size(), base::WriteFile(temp_file_, blob.data(), blob.size())); diff --git a/brillo/make_unique_ptr.h b/brillo/make_unique_ptr.h deleted file mode 100644 index 89f56e6..0000000 --- a/brillo/make_unique_ptr.h +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright 2015 The Chromium OS Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef LIBBRILLO_BRILLO_MAKE_UNIQUE_PTR_H_ -#define LIBBRILLO_BRILLO_MAKE_UNIQUE_PTR_H_ - -#include <memory> - -namespace brillo { - -// A function to convert T* into unique_ptr<T> -// Doing e.g. make_unique_ptr(new FooBarBaz<type>(arg)) is a shorter notation -// for unique_ptr<FooBarBaz<type>>(new FooBarBaz<type>(arg)) -// Basically the same as Chromium's make_scoped_ptr(). -// Deliberately not named "make_unique" to avoid conflicting with the similar, -// but more complex and semantically different C++14 function. -template <typename T> -std::unique_ptr<T> make_unique_ptr(T* ptr) { - return std::unique_ptr<T>(ptr); -} - -} // namespace brillo - -#endif // LIBBRILLO_BRILLO_MAKE_UNIQUE_PTR_H_ diff --git a/brillo/message_loops/fake_message_loop_unittest.cc b/brillo/message_loops/fake_message_loop_unittest.cc index 1f94a4b..7dc54f7 100644 --- a/brillo/message_loops/fake_message_loop_unittest.cc +++ b/brillo/message_loops/fake_message_loop_unittest.cc @@ -95,15 +95,14 @@ TEST_F(FakeMessageLoopTest, WatchFileDescriptorWaits) { Bind([](int* called) { (*called)++; }, base::Unretained(&called))); EXPECT_NE(MessageLoop::kTaskIdNull, task_id); - auto callback = [](FakeMessageLoop* loop) { loop->BreakLoop(); }; - EXPECT_NE( - MessageLoop::kTaskIdNull, - loop_->PostDelayedTask(Bind(callback, base::Unretained(loop_.get())), - TimeDelta::FromSeconds(10))); - EXPECT_NE( - MessageLoop::kTaskIdNull, - loop_->PostDelayedTask(Bind(callback, base::Unretained(loop_.get())), - TimeDelta::FromSeconds(20))); + EXPECT_NE(MessageLoop::kTaskIdNull, + loop_->PostDelayedTask(Bind(&FakeMessageLoop::BreakLoop, + base::Unretained(loop_.get())), + TimeDelta::FromSeconds(10))); + EXPECT_NE(MessageLoop::kTaskIdNull, + loop_->PostDelayedTask(Bind(&FakeMessageLoop::BreakLoop, + base::Unretained(loop_.get())), + TimeDelta::FromSeconds(20))); loop_->Run(); EXPECT_EQ(0, called); diff --git a/brillo/message_loops/glib_message_loop.cc b/brillo/message_loops/glib_message_loop.cc deleted file mode 100644 index 20c271d..0000000 --- a/brillo/message_loops/glib_message_loop.cc +++ /dev/null @@ -1,194 +0,0 @@ -// Copyright 2015 The Chromium OS 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 <brillo/message_loops/glib_message_loop.h> - -#include <fcntl.h> -#include <unistd.h> - -#include <brillo/location_logging.h> - -using base::Closure; - -namespace brillo { - -GlibMessageLoop::GlibMessageLoop() { - loop_ = g_main_loop_new(g_main_context_default(), FALSE); -} - -GlibMessageLoop::~GlibMessageLoop() { - // Cancel all pending tasks when destroying the message loop. - for (const auto& task : tasks_) { - DVLOG_LOC(task.second->location, 1) - << "Removing task_id " << task.second->task_id - << " leaked on GlibMessageLoop, scheduled from this location."; - g_source_remove(task.second->source_id); - } - g_main_loop_unref(loop_); -} - -MessageLoop::TaskId GlibMessageLoop::PostDelayedTask( - const tracked_objects::Location& from_here, - const Closure &task, - base::TimeDelta delay) { - TaskId task_id = NextTaskId(); - // Note: While we store persistent = false in the ScheduledTask object, we - // don't check it in OnRanPostedTask() since it is always false for delayed - // tasks. This is only used for WatchFileDescriptor below. - ScheduledTask* scheduled_task = new ScheduledTask{ - this, from_here, task_id, 0, false, std::move(task)}; - DVLOG_LOC(from_here, 1) << "Scheduling delayed task_id " << task_id - << " to run in " << delay << "."; - scheduled_task->source_id = g_timeout_add_full( - G_PRIORITY_DEFAULT, - delay.InMillisecondsRoundedUp(), - &GlibMessageLoop::OnRanPostedTask, - reinterpret_cast<gpointer>(scheduled_task), - DestroyPostedTask); - tasks_[task_id] = scheduled_task; - return task_id; -} - -MessageLoop::TaskId GlibMessageLoop::WatchFileDescriptor( - const tracked_objects::Location& from_here, - int fd, - WatchMode mode, - bool persistent, - const Closure &task) { - // Quick check to see if the fd is valid. - if (fcntl(fd, F_GETFD) == -1 && errno == EBADF) - return MessageLoop::kTaskIdNull; - - GIOCondition condition = G_IO_NVAL; - switch (mode) { - case MessageLoop::kWatchRead: - condition = static_cast<GIOCondition>(G_IO_IN | G_IO_HUP | G_IO_NVAL); - break; - case MessageLoop::kWatchWrite: - condition = static_cast<GIOCondition>(G_IO_OUT | G_IO_HUP | G_IO_NVAL); - break; - default: - return MessageLoop::kTaskIdNull; - } - - // TODO(deymo): Used g_unix_fd_add_full() instead of g_io_add_watch_full() - // when/if we switch to glib 2.36 or newer so we don't need to create a - // GIOChannel for this. - GIOChannel* io_channel = g_io_channel_unix_new(fd); - if (!io_channel) - return MessageLoop::kTaskIdNull; - GError* error = nullptr; - GIOStatus status = g_io_channel_set_encoding(io_channel, nullptr, &error); - if (status != G_IO_STATUS_NORMAL) { - LOG(ERROR) << "GError(" << error->code << "): " - << (error->message ? error->message : "(unknown)"); - g_error_free(error); - // g_io_channel_set_encoding() documentation states that this should be - // valid in this context (a new io_channel), but enforce the check in - // debug mode. - DCHECK(status == G_IO_STATUS_NORMAL); - return MessageLoop::kTaskIdNull; - } - - TaskId task_id = NextTaskId(); - ScheduledTask* scheduled_task = new ScheduledTask{ - this, from_here, task_id, 0, persistent, std::move(task)}; - scheduled_task->source_id = g_io_add_watch_full( - io_channel, - G_PRIORITY_DEFAULT, - condition, - &GlibMessageLoop::OnWatchedFdReady, - reinterpret_cast<gpointer>(scheduled_task), - DestroyPostedTask); - // g_io_add_watch_full() increases the reference count on the newly created - // io_channel, so we can dereference it now and it will be free'd once the - // source is removed or now if g_io_add_watch_full() failed. - g_io_channel_unref(io_channel); - - DVLOG_LOC(from_here, 1) - << "Watching fd " << fd << " for " - << (mode == MessageLoop::kWatchRead ? "reading" : "writing") - << (persistent ? " persistently" : " just once") - << " as task_id " << task_id - << (scheduled_task->source_id ? " successfully" : " failed."); - - if (!scheduled_task->source_id) { - delete scheduled_task; - return MessageLoop::kTaskIdNull; - } - tasks_[task_id] = scheduled_task; - return task_id; -} - -bool GlibMessageLoop::CancelTask(TaskId task_id) { - if (task_id == kTaskIdNull) - return false; - const auto task = tasks_.find(task_id); - // It is a programmer error to attempt to remove a non-existent source. - if (task == tasks_.end()) - return false; - DVLOG_LOC(task->second->location, 1) - << "Removing task_id " << task_id << " scheduled from this location."; - guint source_id = task->second->source_id; - // We remove here the entry from the tasks_ map, the pointer will be deleted - // by the g_source_remove() call. - tasks_.erase(task); - return g_source_remove(source_id); -} - -bool GlibMessageLoop::RunOnce(bool may_block) { - return g_main_context_iteration(nullptr, may_block); -} - -void GlibMessageLoop::Run() { - g_main_loop_run(loop_); -} - -void GlibMessageLoop::BreakLoop() { - g_main_loop_quit(loop_); -} - -MessageLoop::TaskId GlibMessageLoop::NextTaskId() { - TaskId res; - do { - res = ++last_id_; - // We would run out of memory before we run out of task ids. - } while (!res || tasks_.find(res) != tasks_.end()); - return res; -} - -gboolean GlibMessageLoop::OnRanPostedTask(gpointer user_data) { - ScheduledTask* scheduled_task = reinterpret_cast<ScheduledTask*>(user_data); - DVLOG_LOC(scheduled_task->location, 1) - << "Running delayed task_id " << scheduled_task->task_id - << " scheduled from this location."; - // We only need to remove this task_id from the map. DestroyPostedTask will be - // called with this same |user_data| where we can delete the ScheduledTask. - scheduled_task->loop->tasks_.erase(scheduled_task->task_id); - scheduled_task->closure.Run(); - return FALSE; // Removes the source since a callback can only be called once. -} - -gboolean GlibMessageLoop::OnWatchedFdReady(GIOChannel *source, - GIOCondition condition, - gpointer user_data) { - ScheduledTask* scheduled_task = reinterpret_cast<ScheduledTask*>(user_data); - DVLOG_LOC(scheduled_task->location, 1) - << "Running task_id " << scheduled_task->task_id - << " for watching a file descriptor, scheduled from this location."; - if (!scheduled_task->persistent) { - // We only need to remove this task_id from the map. DestroyPostedTask will - // be called with this same |user_data| where we can delete the - // ScheduledTask. - scheduled_task->loop->tasks_.erase(scheduled_task->task_id); - } - scheduled_task->closure.Run(); - return scheduled_task->persistent; -} - -void GlibMessageLoop::DestroyPostedTask(gpointer user_data) { - delete reinterpret_cast<ScheduledTask*>(user_data); -} - -} // namespace brillo diff --git a/brillo/message_loops/glib_message_loop.h b/brillo/message_loops/glib_message_loop.h deleted file mode 100644 index 50fe2ce..0000000 --- a/brillo/message_loops/glib_message_loop.h +++ /dev/null @@ -1,83 +0,0 @@ -// Copyright 2015 The Chromium OS Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef LIBBRILLO_BRILLO_MESSAGE_LOOPS_GLIB_MESSAGE_LOOP_H_ -#define LIBBRILLO_BRILLO_MESSAGE_LOOPS_GLIB_MESSAGE_LOOP_H_ - -#include <map> -#include <memory> - -#include <base/location.h> -#include <base/time/time.h> -#include <glib.h> - -#include <brillo/brillo_export.h> -#include <brillo/message_loops/message_loop.h> - -namespace brillo { - -class BRILLO_EXPORT GlibMessageLoop : public MessageLoop { - public: - GlibMessageLoop(); - ~GlibMessageLoop() override; - - // MessageLoop overrides. - TaskId PostDelayedTask(const tracked_objects::Location& from_here, - const base::Closure& task, - base::TimeDelta delay) override; - using MessageLoop::PostDelayedTask; - TaskId WatchFileDescriptor(const tracked_objects::Location& from_here, - int fd, - WatchMode mode, - bool persistent, - const base::Closure& task) override; - using MessageLoop::WatchFileDescriptor; - bool CancelTask(TaskId task_id) override; - bool RunOnce(bool may_block) override; - void Run() override; - void BreakLoop() override; - - private: - // Called by the GLib's main loop when is time to call the callback scheduled - // with Post*Task(). The pointer to the callback passed when scheduling it is - // passed to this function as a gpointer on |user_data|. - static gboolean OnRanPostedTask(gpointer user_data); - - // Called by the GLib's main loop when the watched source |source| is - // ready to perform the operation given in |condition| without blocking. - static gboolean OnWatchedFdReady(GIOChannel *source, - GIOCondition condition, - gpointer user_data); - - // Called by the GLib's main loop when the scheduled callback is removed due - // to it being executed or canceled. - static void DestroyPostedTask(gpointer user_data); - - // Return a new unused task_id. - TaskId NextTaskId(); - - GMainLoop* loop_; - - struct ScheduledTask { - // A pointer to this GlibMessageLoop so we can remove the Task from the - // glib callback. - GlibMessageLoop* loop; - tracked_objects::Location location; - - MessageLoop::TaskId task_id; - guint source_id; - bool persistent; - base::Closure closure; - }; - - std::map<MessageLoop::TaskId, ScheduledTask*> tasks_; - - MessageLoop::TaskId last_id_ = kTaskIdNull; - - DISALLOW_COPY_AND_ASSIGN(GlibMessageLoop); -}; - -} // namespace brillo - -#endif // LIBBRILLO_BRILLO_MESSAGE_LOOPS_GLIB_MESSAGE_LOOP_H_ diff --git a/brillo/message_loops/glib_message_loop_unittest.cc b/brillo/message_loops/glib_message_loop_unittest.cc deleted file mode 100644 index 4b72a11..0000000 --- a/brillo/message_loops/glib_message_loop_unittest.cc +++ /dev/null @@ -1,69 +0,0 @@ -// Copyright 2015 The Chromium OS 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 <brillo/message_loops/glib_message_loop.h> - -#include <fcntl.h> -#include <unistd.h> - -#include <memory> - -#include <base/bind.h> -#include <base/location.h> -#include <base/posix/eintr_wrapper.h> -#include <gtest/gtest.h> - -#include <brillo/bind_lambda.h> -#include <brillo/message_loops/message_loop.h> -#include <brillo/message_loops/message_loop_utils.h> - -using base::Bind; - -namespace brillo { - -using TaskId = MessageLoop::TaskId; - -class GlibMessageLoopTest : public ::testing::Test { - protected: - void SetUp() override { - loop_.reset(new GlibMessageLoop()); - EXPECT_TRUE(loop_.get()); - } - - std::unique_ptr<GlibMessageLoop> loop_; -}; - -// When you watch a file descriptor for reading, the guaranties are that a -// blocking call to read() on that file descriptor will not block. This should -// include the case when the other end of a pipe is closed or the file is empty. -TEST_F(GlibMessageLoopTest, WatchFileDescriptorTriggersWhenEmpty) { - int fd = HANDLE_EINTR(open("/dev/null", O_RDONLY)); - int called = 0; - TaskId task_id = loop_->WatchFileDescriptor( - FROM_HERE, fd, MessageLoop::kWatchRead, true, - Bind([&called] { called++; })); - EXPECT_NE(MessageLoop::kTaskIdNull, task_id); - EXPECT_NE(0, MessageLoopRunMaxIterations(loop_.get(), 10)); - EXPECT_LT(2, called); - EXPECT_TRUE(loop_->CancelTask(task_id)); -} - -// Test that an invalid file descriptor triggers the callback. -TEST_F(GlibMessageLoopTest, WatchFileDescriptorTriggersWhenInvalid) { - int fd = HANDLE_EINTR(open("/dev/zero", O_RDONLY)); - int called = 0; - TaskId task_id = loop_->WatchFileDescriptor( - FROM_HERE, fd, MessageLoop::kWatchRead, true, - Bind([&called, fd] { - if (!called) - IGNORE_EINTR(close(fd)); - called++; - })); - EXPECT_NE(MessageLoop::kTaskIdNull, task_id); - EXPECT_NE(0, MessageLoopRunMaxIterations(loop_.get(), 10)); - EXPECT_LT(2, called); - EXPECT_TRUE(loop_->CancelTask(task_id)); -} - -} // namespace brillo diff --git a/brillo/message_loops/message_loop_unittest.cc b/brillo/message_loops/message_loop_unittest.cc index 0265fbf..8024256 100644 --- a/brillo/message_loops/message_loop_unittest.cc +++ b/brillo/message_loops/message_loop_unittest.cc @@ -20,12 +20,28 @@ #include <brillo/bind_lambda.h> #include <brillo/unittest_utils.h> #include <brillo/message_loops/base_message_loop.h> -#include <brillo/message_loops/glib_message_loop.h> #include <brillo/message_loops/message_loop_utils.h> using base::Bind; using base::TimeDelta; +namespace { + +// Convenience functions for passing to base::Bind. +void SetToTrue(bool* b) { + *b = true; +} + +bool ReturnBool(bool *b) { + return *b; +} + +void Increment(int* i) { + (*i)++; +} + +} // namespace + namespace brillo { using TaskId = MessageLoop::TaskId; @@ -49,11 +65,6 @@ class MessageLoopTest : public ::testing::Test { }; template <> -void MessageLoopTest<GlibMessageLoop>::MessageLoopSetUp() { - loop_.reset(new GlibMessageLoop()); -} - -template <> void MessageLoopTest<BaseMessageLoop>::MessageLoopSetUp() { base_loop_.reset(new base::MessageLoopForIO()); loop_.reset(new BaseMessageLoop(base::MessageLoopForIO::current())); @@ -61,9 +72,7 @@ void MessageLoopTest<BaseMessageLoop>::MessageLoopSetUp() { // This setups gtest to run each one of the following TYPED_TEST test cases on // on each implementation. -typedef ::testing::Types< - GlibMessageLoop, - BaseMessageLoop> MessageLoopTypes; +typedef ::testing::Types<BaseMessageLoop> MessageLoopTypes; TYPED_TEST_CASE(MessageLoopTest, MessageLoopTypes); @@ -74,8 +83,7 @@ TYPED_TEST(MessageLoopTest, CancelTaskInvalidValuesTest) { TYPED_TEST(MessageLoopTest, PostTaskTest) { bool called = false; - TaskId task_id = this->loop_->PostTask(FROM_HERE, - Bind([&called]() { called = true; })); + TaskId task_id = this->loop_->PostTask(FROM_HERE, Bind(&SetToTrue, &called)); EXPECT_NE(MessageLoop::kTaskIdNull, task_id); MessageLoopRunMaxIterations(this->loop_.get(), 100); EXPECT_TRUE(called); @@ -84,8 +92,7 @@ TYPED_TEST(MessageLoopTest, PostTaskTest) { // Tests that we can cancel tasks right after we schedule them. TYPED_TEST(MessageLoopTest, PostTaskCancelledTest) { bool called = false; - TaskId task_id = this->loop_->PostTask(FROM_HERE, - Bind([&called]() { called = true; })); + TaskId task_id = this->loop_->PostTask(FROM_HERE, Bind(&SetToTrue, &called)); EXPECT_TRUE(this->loop_->CancelTask(task_id)); MessageLoopRunMaxIterations(this->loop_.get(), 100); EXPECT_FALSE(called); @@ -96,13 +103,11 @@ TYPED_TEST(MessageLoopTest, PostTaskCancelledTest) { TYPED_TEST(MessageLoopTest, PostDelayedTaskRunsEventuallyTest) { bool called = false; TaskId task_id = this->loop_->PostDelayedTask( - FROM_HERE, - Bind([&called]() { called = true; }), - TimeDelta::FromMilliseconds(50)); + FROM_HERE, Bind(&SetToTrue, &called), TimeDelta::FromMilliseconds(50)); EXPECT_NE(MessageLoop::kTaskIdNull, task_id); MessageLoopRunUntil(this->loop_.get(), TimeDelta::FromSeconds(10), - Bind([&called]() { return called; })); + Bind(&ReturnBool, &called)); // Check that the main loop finished before the 10 seconds timeout, so it // finished due to the callback being called and not due to the timeout. EXPECT_TRUE(called); @@ -120,10 +125,10 @@ TYPED_TEST(MessageLoopTest, WatchForInvalidFD) { bool called = false; EXPECT_EQ(MessageLoop::kTaskIdNull, this->loop_->WatchFileDescriptor( FROM_HERE, -1, MessageLoop::kWatchRead, true, - Bind([&called] { called = true; }))); + Bind(&SetToTrue, &called))); EXPECT_EQ(MessageLoop::kTaskIdNull, this->loop_->WatchFileDescriptor( FROM_HERE, -1, MessageLoop::kWatchWrite, true, - Bind([&called] { called = true; }))); + Bind(&SetToTrue, &called))); EXPECT_EQ(0, MessageLoopRunMaxIterations(this->loop_.get(), 100)); EXPECT_FALSE(called); } @@ -133,7 +138,7 @@ TYPED_TEST(MessageLoopTest, CancelWatchedFileDescriptor) { bool called = false; TaskId task_id = this->loop_->WatchFileDescriptor( FROM_HERE, pipe.reader, MessageLoop::kWatchRead, true, - Bind([&called] { called = true; })); + Bind(&SetToTrue, &called)); EXPECT_NE(MessageLoop::kTaskIdNull, task_id); // The reader end is blocked because we didn't write anything to the writer // end. @@ -152,7 +157,7 @@ TYPED_TEST(MessageLoopTest, WatchFileDescriptorTriggersWhenPipeClosed) { pipe.writer = -1; TaskId task_id = this->loop_->WatchFileDescriptor( FROM_HERE, pipe.reader, MessageLoop::kWatchRead, true, - Bind([&called] { called = true; })); + Bind(&SetToTrue, &called)); EXPECT_NE(MessageLoop::kTaskIdNull, task_id); // The reader end is not blocked because we closed the writer end so a read on // the reader end would return 0 bytes read. @@ -170,7 +175,7 @@ TYPED_TEST(MessageLoopTest, WatchFileDescriptorPersistently) { int called = 0; TaskId task_id = this->loop_->WatchFileDescriptor( FROM_HERE, pipe.reader, MessageLoop::kWatchRead, true, - Bind([&called] { called++; })); + Bind(&Increment, &called)); EXPECT_NE(MessageLoop::kTaskIdNull, task_id); // We let the main loop run for 20 iterations to give it enough iterations to // verify that our callback was called more than one. We only check that our @@ -187,7 +192,7 @@ TYPED_TEST(MessageLoopTest, WatchFileDescriptorNonPersistent) { int called = 0; TaskId task_id = this->loop_->WatchFileDescriptor( FROM_HERE, pipe.reader, MessageLoop::kWatchRead, false, - Bind([&called] { called++; })); + Bind(&Increment, &called)); EXPECT_NE(MessageLoop::kTaskIdNull, task_id); // We let the main loop run for 20 iterations but we just expect it to run // at least once. The callback should be called exactly once since we @@ -206,17 +211,17 @@ TYPED_TEST(MessageLoopTest, WatchFileDescriptorForReadAndWriteSimultaneously) { TaskId read_task_id = this->loop_->WatchFileDescriptor( FROM_HERE, socks.left, MessageLoop::kWatchRead, true, - Bind([this, &read_task_id] { - EXPECT_TRUE(this->loop_->CancelTask(read_task_id)) - << "task_id" << read_task_id; - })); + Bind([] (MessageLoop* loop, TaskId* read_task_id) { + EXPECT_TRUE(loop->CancelTask(*read_task_id)) + << "task_id" << *read_task_id; + }, this->loop_.get(), &read_task_id)); EXPECT_NE(MessageLoop::kTaskIdNull, read_task_id); TaskId write_task_id = this->loop_->WatchFileDescriptor( FROM_HERE, socks.left, MessageLoop::kWatchWrite, true, - Bind([this, &write_task_id] { - EXPECT_TRUE(this->loop_->CancelTask(write_task_id)); - })); + Bind([] (MessageLoop* loop, TaskId* write_task_id) { + EXPECT_TRUE(loop->CancelTask(*write_task_id)); + }, this->loop_.get(), &write_task_id)); EXPECT_NE(MessageLoop::kTaskIdNull, write_task_id); EXPECT_LT(0, MessageLoopRunMaxIterations(this->loop_.get(), 20)); @@ -228,13 +233,12 @@ TYPED_TEST(MessageLoopTest, WatchFileDescriptorForReadAndWriteSimultaneously) { // Test that we can cancel the task we are running, and should just fail. TYPED_TEST(MessageLoopTest, DeleteTaskFromSelf) { bool cancel_result = true; // We would expect this to be false. - MessageLoop* loop_ptr = this->loop_.get(); TaskId task_id; task_id = this->loop_->PostTask( FROM_HERE, - Bind([&cancel_result, loop_ptr, &task_id]() { - cancel_result = loop_ptr->CancelTask(task_id); - })); + Bind([](bool* cancel_result, MessageLoop* loop, TaskId* task_id) { + *cancel_result = loop->CancelTask(*task_id); + }, &cancel_result, this->loop_.get(), &task_id)); EXPECT_EQ(1, MessageLoopRunMaxIterations(this->loop_.get(), 100)); EXPECT_FALSE(cancel_result); } @@ -245,10 +249,10 @@ TYPED_TEST(MessageLoopTest, DeleteNonPersistenIOTaskFromSelf) { ScopedPipe pipe; TaskId task_id = this->loop_->WatchFileDescriptor( FROM_HERE, pipe.writer, MessageLoop::kWatchWrite, false /* persistent */, - Bind([this, &task_id] { - EXPECT_FALSE(this->loop_->CancelTask(task_id)); - task_id = MessageLoop::kTaskIdNull; - })); + Bind([](MessageLoop* loop, TaskId* task_id) { + EXPECT_FALSE(loop->CancelTask(*task_id)); + *task_id = MessageLoop::kTaskIdNull; + }, this->loop_.get(), &task_id)); EXPECT_NE(MessageLoop::kTaskIdNull, task_id); EXPECT_EQ(1, MessageLoopRunMaxIterations(this->loop_.get(), 100)); EXPECT_EQ(MessageLoop::kTaskIdNull, task_id); @@ -260,10 +264,10 @@ TYPED_TEST(MessageLoopTest, DeletePersistenIOTaskFromSelf) { ScopedPipe pipe; TaskId task_id = this->loop_->WatchFileDescriptor( FROM_HERE, pipe.writer, MessageLoop::kWatchWrite, true /* persistent */, - Bind([this, &task_id] { - EXPECT_TRUE(this->loop_->CancelTask(task_id)); - task_id = MessageLoop::kTaskIdNull; - })); + Bind([](MessageLoop* loop, TaskId* task_id) { + EXPECT_TRUE(loop->CancelTask(*task_id)); + *task_id = MessageLoop::kTaskIdNull; + }, this->loop_.get(), &task_id)); EXPECT_NE(MessageLoop::kTaskIdNull, task_id); EXPECT_EQ(1, MessageLoopRunMaxIterations(this->loop_.get(), 100)); EXPECT_EQ(MessageLoop::kTaskIdNull, task_id); @@ -282,14 +286,14 @@ TYPED_TEST(MessageLoopTest, DeleteAllPersistenIOTaskFromSelf) { task_ids[i] = this->loop_->WatchFileDescriptor( FROM_HERE, pipes[i].writer, MessageLoop::kWatchWrite, true /* persistent */, - Bind([this, &task_ids] { + Bind([] (MessageLoop* loop, TaskId* task_ids) { for (int j = 0; j < kNumTasks; ++j) { // Once we cancel all the tasks, none should run, so this code runs // only once from one callback. - EXPECT_TRUE(this->loop_->CancelTask(task_ids[j])); + EXPECT_TRUE(loop->CancelTask(task_ids[j])); task_ids[j] = MessageLoop::kTaskIdNull; } - })); + }, this->loop_.get(), task_ids)); } MessageLoopRunMaxIterations(this->loop_.get(), 100); for (int i = 0; i < kNumTasks; ++i) { @@ -310,13 +314,16 @@ TYPED_TEST(MessageLoopTest, AllTasksAreEqual) { base::Closure timeout_callback; MessageLoop::TaskId timeout_task; timeout_callback = base::Bind( - [this, &timeout_called, &total_calls, &timeout_callback, &timeout_task] { - timeout_called++; - total_calls++; - timeout_task = this->loop_->PostTask(FROM_HERE, Bind(timeout_callback)); - if (total_calls > 100) - this->loop_->BreakLoop(); - }); + [](MessageLoop* loop, int* timeout_called, int* total_calls, + base::Closure* timeout_callback, MessageLoop::TaskId* timeout_task) { + (*timeout_called)++; + (*total_calls)++; + *timeout_task = loop->PostTask(FROM_HERE, *timeout_callback); + if (*total_calls > 100) + loop->BreakLoop(); + }, + this->loop_.get(), &timeout_called, &total_calls, &timeout_callback, + &timeout_task); timeout_task = this->loop_->PostTask(FROM_HERE, timeout_callback); // Second, schedule several file descriptor watchers. @@ -325,14 +332,16 @@ TYPED_TEST(MessageLoopTest, AllTasksAreEqual) { MessageLoop::TaskId tasks[kNumTasks]; int reads[kNumTasks] = {}; - auto fd_callback = [this, &pipes, &reads, &total_calls](int i) { + base::Callback<void(int)> fd_callback = base::Bind( + [](MessageLoop* loop, ScopedPipe* pipes, int* reads, + int* total_calls, int i) { reads[i]++; - total_calls++; + (*total_calls)++; char c; EXPECT_EQ(1, HANDLE_EINTR(read(pipes[i].reader, &c, 1))); - if (total_calls > 100) - this->loop_->BreakLoop(); - }; + if (*total_calls > 100) + loop->BreakLoop(); + }, this->loop_.get(), pipes, reads, &total_calls); for (int i = 0; i < kNumTasks; ++i) { tasks[i] = this->loop_->WatchFileDescriptor( diff --git a/brillo/process.cc b/brillo/process.cc index 7af8fbb..8773244 100644 --- a/brillo/process.cc +++ b/brillo/process.cc @@ -15,6 +15,7 @@ #include <map> #include <memory> +#include <base/files/file_path.h> #include <base/files/file_util.h> #include <base/logging.h> #include <base/memory/ptr_util.h> @@ -64,6 +65,10 @@ void ProcessImpl::AddArg(const std::string& arg) { arguments_.push_back(arg); } +void ProcessImpl::RedirectInput(const std::string& input_file) { + input_file_ = input_file; +} + void ProcessImpl::RedirectOutput(const std::string& output_file) { output_file_ = output_file; } @@ -212,7 +217,7 @@ bool ProcessImpl::Start() { return false; } std::unique_ptr<char* []> argv = - base::MakeUnique<char* []>(arguments_.size() + 1); + std::make_unique<char* []>(arguments_.size() + 1); for (size_t i = 0; i < arguments_.size(); ++i) argv[i] = const_cast<char*>(arguments_[i].c_str()); @@ -257,6 +262,28 @@ bool ProcessImpl::Start() { continue; IGNORE_EINTR(close(i->second.child_fd_)); } + + if (!input_file_.empty()) { + int input_handle = + HANDLE_EINTR(open(input_file_.c_str(), + O_RDONLY | O_NOFOLLOW | O_NOCTTY)); + if (input_handle < 0) { + PLOG(ERROR) << "Could not open " << input_file_; + // Avoid exit() to avoid atexit handlers from parent. + _exit(kErrorExitStatus); + } + + // It's possible input_handle is already stdin. But if not, we need + // to dup into that file descriptor and close the original. + if (input_handle != STDIN_FILENO) { + if (HANDLE_EINTR(dup2(input_handle, STDIN_FILENO)) < 0) { + PLOG(ERROR) << "Could not dup fd to stdin for " << input_file_; + _exit(kErrorExitStatus); + } + IGNORE_EINTR(close(input_handle)); + } + } + if (!output_file_.empty()) { int output_handle = HANDLE_EINTR(open( output_file_.c_str(), O_CREAT | O_WRONLY | O_TRUNC | O_NOFOLLOW, diff --git a/brillo/process.h b/brillo/process.h index 9760ab3..d69d38b 100644 --- a/brillo/process.h +++ b/brillo/process.h @@ -13,6 +13,7 @@ #include <base/bind.h> #include <base/callback.h> +#include <base/files/file_path.h> #include <base/strings/string_util.h> #include <base/strings/stringprintf.h> #include <brillo/brillo_export.h> @@ -48,7 +49,12 @@ class BRILLO_EXPORT Process { AddArg(base::StringPrintf("%d", value)); } - // Redirects stderr and stdout to |output_file|. + // Redirects to read stdin from |input_file|. |input_file| must not be + // a symlink. + virtual void RedirectInput(const std::string& input_file) = 0; + + // Redirects stderr and stdout to |output_file|. |output_file| must not be + // a symlink. virtual void RedirectOutput(const std::string& output_file) = 0; // Indicates we want to redirect |child_fd| in the child process's @@ -163,6 +169,7 @@ class BRILLO_EXPORT ProcessImpl : public Process { virtual ~ProcessImpl(); virtual void AddArg(const std::string& arg); + virtual void RedirectInput(const std::string& input_file); virtual void RedirectOutput(const std::string& output_file); virtual void RedirectUsingPipe(int child_fd, bool is_input); virtual void BindFd(int parent_fd, int child_fd); @@ -212,6 +219,7 @@ class BRILLO_EXPORT ProcessImpl : public Process { // process. pid must not be modified except by calling // UpdatePid(new_pid). pid_t pid_; + std::string input_file_; std::string output_file_; std::vector<std::string> arguments_; // Map of child target file descriptors (first) to information about diff --git a/brillo/process_mock.h b/brillo/process_mock.h index f73d242..92ffa0a 100644 --- a/brillo/process_mock.h +++ b/brillo/process_mock.h @@ -7,6 +7,7 @@ #include <string> +#include <base/files/file_path.h> #include <gmock/gmock.h> #include "brillo/process.h" @@ -19,6 +20,7 @@ class ProcessMock : public Process { virtual ~ProcessMock() {} MOCK_METHOD1(AddArg, void(const std::string& arg)); + MOCK_METHOD1(RedirectInput, void(const std::string& input_file)); MOCK_METHOD1(RedirectOutput, void(const std::string& output_file)); MOCK_METHOD2(RedirectUsingPipe, void(int child_fd, bool is_input)); MOCK_METHOD2(BindFd, void(int parent_fd, int child_fd)); diff --git a/brillo/process_reaper.cc b/brillo/process_reaper.cc index c4cb1cf..3d311c4 100644 --- a/brillo/process_reaper.cc +++ b/brillo/process_reaper.cc @@ -11,7 +11,6 @@ #include <base/bind.h> #include <base/posix/eintr_wrapper.h> #include <brillo/asynchronous_signal_handler.h> -#include <brillo/daemons/daemon.h> #include <brillo/location_logging.h> namespace brillo { diff --git a/brillo/process_reaper.h b/brillo/process_reaper.h index 937c88c..41bbe73 100644 --- a/brillo/process_reaper.h +++ b/brillo/process_reaper.h @@ -13,7 +13,6 @@ #include <base/location.h> #include <base/macros.h> #include <brillo/asynchronous_signal_handler.h> -#include <brillo/daemons/daemon.h> namespace brillo { diff --git a/brillo/process_reaper_unittest.cc b/brillo/process_reaper_unittest.cc index 0ca6e34..98498f7 100644 --- a/brillo/process_reaper_unittest.cc +++ b/brillo/process_reaper_unittest.cc @@ -75,65 +75,57 @@ TEST_F(ProcessReaperTest, UnregisterAndReregister) { TEST_F(ProcessReaperTest, ReapExitedChild) { pid_t pid = ForkChildAndExit(123); EXPECT_TRUE(process_reaper_.WatchForChild(FROM_HERE, pid, base::Bind( - [](decltype(this) test, const siginfo_t& info) { + [](MessageLoop* loop, const siginfo_t& info) { EXPECT_EQ(CLD_EXITED, info.si_code); EXPECT_EQ(123, info.si_status); - test->brillo_loop_.BreakLoop(); - }, base::Unretained(this)))); + loop->BreakLoop(); + }, &brillo_loop_))); brillo_loop_.Run(); } // Test that simultaneous child processes fire their respective callbacks when // exiting. -TEST_F(ProcessReaperTest, ReapedChildsMatchCallbacks) { - int running_childs = 10; - for (int i = 0; i < running_childs; ++i) { +TEST_F(ProcessReaperTest, ReapedChildrenMatchCallbacks) { + int running_children = 10; + for (int i = 0; i < running_children; ++i) { // Different processes will have different exit values. int exit_value = 1 + i; pid_t pid = ForkChildAndExit(exit_value); - EXPECT_TRUE(process_reaper_.WatchForChild( - FROM_HERE, - pid, - base::Bind( - [](decltype(this) test, - int exit_value, - int* running_childs, - const siginfo_t& info) { - EXPECT_EQ(CLD_EXITED, info.si_code); - EXPECT_EQ(exit_value, info.si_status); - (*running_childs)--; - if (*running_childs == 0) - test->brillo_loop_.BreakLoop(); - }, - base::Unretained(this), - exit_value, - base::Unretained(&running_childs)))); + EXPECT_TRUE(process_reaper_.WatchForChild(FROM_HERE, pid, base::Bind( + [](MessageLoop* loop, int exit_value, int* running_children, + const siginfo_t& info) { + EXPECT_EQ(CLD_EXITED, info.si_code); + EXPECT_EQ(exit_value, info.si_status); + (*running_children)--; + if (*running_children == 0) + loop->BreakLoop(); + }, &brillo_loop_, exit_value, &running_children))); } // This sleep is optional. It helps to have more processes exit before we // start watching for them in the message loop. usleep(10 * 1000); brillo_loop_.Run(); - EXPECT_EQ(0, running_childs); + EXPECT_EQ(0, running_children); } TEST_F(ProcessReaperTest, ReapKilledChild) { pid_t pid = ForkChildAndKill(SIGKILL); EXPECT_TRUE(process_reaper_.WatchForChild(FROM_HERE, pid, base::Bind( - [](decltype(this) test, const siginfo_t& info) { + [](MessageLoop* loop, const siginfo_t& info) { EXPECT_EQ(CLD_KILLED, info.si_code); EXPECT_EQ(SIGKILL, info.si_status); - test->brillo_loop_.BreakLoop(); - }, base::Unretained(this)))); + loop->BreakLoop(); + }, &brillo_loop_))); brillo_loop_.Run(); } TEST_F(ProcessReaperTest, ReapKilledAndForgottenChild) { pid_t pid = ForkChildAndExit(0); EXPECT_TRUE(process_reaper_.WatchForChild(FROM_HERE, pid, base::Bind( - [](decltype(this) test, const siginfo_t& /* info */) { + [](MessageLoop* loop, const siginfo_t& /* info */) { ADD_FAILURE() << "Child process was still tracked."; - test->brillo_loop_.BreakLoop(); - }, base::Unretained(this)))); + loop->BreakLoop(); + }, &brillo_loop_))); EXPECT_TRUE(process_reaper_.ForgetChild(pid)); // A second call should return failure. diff --git a/brillo/process_unittest.cc b/brillo/process_unittest.cc index 20ccf2e..d980e2b 100644 --- a/brillo/process_unittest.cc +++ b/brillo/process_unittest.cc @@ -157,6 +157,18 @@ TEST_F(ProcessTest, NonZeroReturnValue) { EXPECT_EQ("", GetLog()); } +TEST_F(ProcessTest, RedirectInputDevNull) { + process_.AddArg(kBinCat); + process_.RedirectInput("/dev/null"); + EXPECT_EQ(0, process_.Run()); +} + +TEST_F(ProcessTest, BadInputFile) { + process_.AddArg(kBinCat); + process_.RedirectInput("/bad/path"); + EXPECT_EQ(static_cast<pid_t>(Process::kErrorExitStatus), process_.Run()); +} + TEST_F(ProcessTest, BadOutputFile) { process_.AddArg(kBinEcho); process_.RedirectOutput("/bad/path"); diff --git a/brillo/secure_blob.cc b/brillo/secure_blob.cc index 9e6d570..f4b797f 100644 --- a/brillo/secure_blob.cc +++ b/brillo/secure_blob.cc @@ -5,11 +5,34 @@ #include <cstring> // memcpy #include <base/stl_util.h> +#include <base/strings/string_number_conversions.h> #include "brillo/secure_blob.h" namespace brillo { +std::string BlobToString(const Blob& blob) { + return std::string(blob.begin(), blob.end()); +} + +Blob BlobFromString(const std::string& bytes) { + return Blob(bytes.begin(), bytes.end()); +} + +Blob CombineBlobs(const std::initializer_list<Blob>& blobs) { + size_t total_size = 0; + for (const auto& blob : blobs) + total_size += blob.size(); + Blob concatenation; + concatenation.reserve(total_size); + for (const auto& blob : blobs) + concatenation.insert(concatenation.end(), blob.begin(), blob.end()); + return concatenation; +} + +SecureBlob::SecureBlob(const Blob& blob) + : SecureBlob(blob.begin(), blob.end()) {} + SecureBlob::SecureBlob(const std::string& data) : SecureBlob(data.begin(), data.end()) {} @@ -49,7 +72,21 @@ SecureBlob SecureBlob::Combine(const SecureBlob& blob1, return result; } -void* SecureMemset(void* v, int c, size_t n) { +bool SecureBlob::HexStringToSecureBlob(const std::string& input, + SecureBlob* output) { + // TODO(jorgelo,crbug.com/728047): Consider not using an intermediate + // std::vector here at all. + std::vector<uint8_t> temp; + if (!base::HexStringToBytes(input, &temp)) { + output->clear(); + return false; + } + output->assign(temp.begin(), temp.end()); + SecureMemset(temp.data(), 0, temp.capacity()); + return true; +} + +BRILLO_DISABLE_ASAN void* SecureMemset(void* v, int c, size_t n) { volatile uint8_t* p = reinterpret_cast<volatile uint8_t*>(v); while (n--) *p++ = c; diff --git a/brillo/secure_blob.h b/brillo/secure_blob.h index b6111c7..7b6d03c 100644 --- a/brillo/secure_blob.h +++ b/brillo/secure_blob.h @@ -5,21 +5,32 @@ #ifndef LIBBRILLO_BRILLO_SECURE_BLOB_H_ #define LIBBRILLO_BRILLO_SECURE_BLOB_H_ +#include <initializer_list> #include <string> #include <vector> +#include <brillo/asan.h> #include <brillo/brillo_export.h> namespace brillo { using Blob = std::vector<uint8_t>; +// Conversion of Blob to/from std::string, where the string holds raw byte +// contents. +BRILLO_EXPORT std::string BlobToString(const Blob& blob); +BRILLO_EXPORT Blob BlobFromString(const std::string& bytes); + +// Returns a concatenation of given Blobs. +BRILLO_EXPORT Blob CombineBlobs(const std::initializer_list<Blob>& blobs); + // SecureBlob erases the contents on destruction. It does not guarantee erasure // on resize, assign, etc. class BRILLO_EXPORT SecureBlob : public Blob { public: SecureBlob() = default; using Blob::vector; // Inherit standard constructors from vector. + explicit SecureBlob(const Blob& blob); explicit SecureBlob(const std::string& data); ~SecureBlob(); @@ -33,6 +44,8 @@ class BRILLO_EXPORT SecureBlob : public Blob { return reinterpret_cast<const char*>(data()); } static SecureBlob Combine(const SecureBlob& blob1, const SecureBlob& blob2); + static bool HexStringToSecureBlob(const std::string& input, + SecureBlob* output); }; // Secure memset(). This function is guaranteed to fill in the whole buffer @@ -46,7 +59,10 @@ class BRILLO_EXPORT SecureBlob : public Blob { // While memset() can be optimized out in certain situations (since most // compilers implement this function as intrinsic and know of its side effects), // this function will not be optimized out. -BRILLO_EXPORT void* SecureMemset(void* v, int c, size_t n); +// +// SecureMemset is used to write beyond the size() in several functions. +// Since this is intentional, disable address sanitizer from analying it. +BRILLO_EXPORT BRILLO_DISABLE_ASAN void* SecureMemset(void* v, int c, size_t n); // Compare [n] bytes starting at [s1] with [s2] and return 0 if they match, // 1 if they don't. Time taken to perform the comparison is only dependent on diff --git a/brillo/secure_blob_unittest.cc b/brillo/secure_blob_unittest.cc index d4fd555..ff95d0f 100644 --- a/brillo/secure_blob_unittest.cc +++ b/brillo/secure_blob_unittest.cc @@ -4,10 +4,12 @@ // Unit tests for SecureBlob. +#include "brillo/asan.h" #include "brillo/secure_blob.h" #include <algorithm> #include <iterator> +#include <limits> #include <numeric> #include <base/logging.h> @@ -16,20 +18,47 @@ namespace brillo { using std::string; +// Tests BlobToString() and BlobFromString(). +TEST(BlobTest, StringConversions) { + const char kTestBytes[] = {'\0', '\x1', 'a', std::numeric_limits<char>::min(), + std::numeric_limits<char>::max()}; + const Blob blob(std::begin(kTestBytes), std::end(kTestBytes)); + const string obtained_string = BlobToString(blob); + EXPECT_EQ(string(std::begin(kTestBytes), std::end(kTestBytes)), + obtained_string); + const Blob obtained_blob = BlobFromString(obtained_string); + EXPECT_EQ(blob, obtained_blob); +} + +// Tests CombineBlobs(). +TEST(BlobTest, CombineBlobs) { + const Blob kEmpty; + const Blob kBlob1 = {1}; + const Blob kBlob2 = {2}; + const Blob kBlob3 = {3}; + const Blob kBlob12 = {1, 2}; + const Blob kBlob123 = {1, 2, 3}; + EXPECT_EQ(kBlob123, CombineBlobs({kBlob12, kBlob3})); + EXPECT_EQ(kBlob123, CombineBlobs({kBlob1, kBlob2, kBlob3})); + EXPECT_EQ(kBlob12, CombineBlobs({kBlob12})); + EXPECT_EQ(kBlob12, CombineBlobs({kEmpty, kBlob1, kEmpty, kBlob2, kEmpty})); + EXPECT_EQ(kEmpty, CombineBlobs({})); +} + class SecureBlobTest : public ::testing::Test { public: SecureBlobTest() {} virtual ~SecureBlobTest() {} - static bool FindBlobInBlob(const brillo::Blob& haystack, - const brillo::Blob& needle) { + static bool FindBlobInBlob(const brillo::SecureBlob& haystack, + const brillo::SecureBlob& needle) { auto pos = std::search( haystack.begin(), haystack.end(), needle.begin(), needle.end()); return (pos != haystack.end()); } - static int FindBlobIndexInBlob(const brillo::Blob& haystack, - const brillo::Blob& needle) { + static int FindBlobIndexInBlob(const brillo::SecureBlob& haystack, + const brillo::SecureBlob& needle) { auto pos = std::search( haystack.begin(), haystack.end(), needle.begin(), needle.end()); if (pos == haystack.end()) { @@ -43,14 +72,32 @@ class SecureBlobTest : public ::testing::Test { }; TEST_F(SecureBlobTest, AllocationSizeTest) { - // Check that allocating a SecureBlob of a specified size works + // Checks that allocating a SecureBlob of a specified size works. SecureBlob blob(32); EXPECT_EQ(32, blob.size()); } -TEST_F(SecureBlobTest, AllocationCopyTest) { - // Check that allocating a SecureBlob with an iterator works +TEST_F(SecureBlobTest, ConstructorCountValueTest) { + // Checks that constructing a SecureBlob with |count| copies of |value| works. + SecureBlob blob(32, 'a'); + + for (size_t i = 0; i < blob.size(); i++) { + EXPECT_EQ('a', blob[i]); + } +} + +TEST_F(SecureBlobTest, ConstructorAmbiguousTest) { + // This test will become important once SecureBlob stops inheriting from Blob. + SecureBlob blob(32, 0); + + for (size_t i = 0; i < blob.size(); i++) { + EXPECT_EQ(0, blob[i]); + } +} + +TEST_F(SecureBlobTest, ConstructorIteratorTest) { + // Checks that constructing a SecureBlob with an iterator works. unsigned char from_data[32]; std::iota(std::begin(from_data), std::end(from_data), 0); @@ -63,23 +110,60 @@ TEST_F(SecureBlobTest, AllocationCopyTest) { } } -TEST_F(SecureBlobTest, IteratorConstructorTest) { - // Check that allocating a SecureBlob with an iterator works - brillo::Blob from_blob(32); - for (unsigned int i = 0; i < from_blob.size(); i++) { - from_blob[i] = i; +TEST_F(SecureBlobTest, BlobConstructorTest) { + // Check that constructing a SecureBlob from a Blob works. + const std::vector<uint8_t> bytes = {0, 1, 255}; + const Blob blob(bytes); + const SecureBlob secure_blob(blob); + EXPECT_EQ(bytes, + std::vector<uint8_t>(secure_blob.begin(), secure_blob.end())); +} + +TEST_F(SecureBlobTest, IteratorTest) { + // Checks that SecureBlob::begin(), SecureBlob::end() work. + unsigned char from_data[32]; + std::iota(std::begin(from_data), std::end(from_data), 0); + + SecureBlob blob(std::begin(from_data), std::end(from_data)); + + EXPECT_EQ(sizeof(from_data), blob.size()); + + size_t i = 0; + for (auto it = blob.begin(); it != blob.end(); ++it) { + EXPECT_EQ(from_data[i], *it); + ++i; + } +} + +TEST_F(SecureBlobTest, AssignTest) { + // Checks that .assign() works. + unsigned char from_data[32]; + std::iota(std::begin(from_data), std::end(from_data), 0); + + SecureBlob blob; + blob.assign(std::begin(from_data), std::end(from_data)); + + EXPECT_EQ(sizeof(from_data), blob.size()); + + size_t i = 0; + for (auto it = blob.begin(); it != blob.end(); ++it) { + EXPECT_EQ(from_data[i], *it); + ++i; } - SecureBlob blob(from_blob.begin(), from_blob.end()); + SecureBlob blob2; + blob2.assign(blob.begin(), blob.end()); - EXPECT_EQ(from_blob.size(), blob.size()); - EXPECT_TRUE(SecureBlobTest::FindBlobInBlob(from_blob, blob)); + EXPECT_EQ(blob, blob2); } +// Disable ResizeTest with Address Sanitizer. +// https://crbug.com/806013 +#ifndef BRILLO_ASAN_BUILD TEST_F(SecureBlobTest, ResizeTest) { - // Check that resizing a SecureBlob wipes the excess memory. The test assumes + // Check that resizing a SecureBlob wipes the excess memory. The test assumes // that resize() down by one will not re-allocate the memory, so the last byte - // will still be part of the SecureBlob's allocation + // will still be part of the SecureBlob's allocation. size_t length = 1024; SecureBlob blob(length); void* original_data = blob.data(); @@ -93,6 +177,7 @@ TEST_F(SecureBlobTest, ResizeTest) { EXPECT_EQ(length - 1, blob.size()); EXPECT_EQ(0, blob.data()[length - 1]); } +#endif TEST_F(SecureBlobTest, CombineTest) { SecureBlob blob1(32); @@ -117,4 +202,29 @@ TEST_F(SecureBlobTest, BlobToStringTest) { EXPECT_EQ(test_string.compare(result_string), 0); } +TEST_F(SecureBlobTest, HexStringToSecureBlob) { + std::string hex_string("112233445566778899aabbccddeeff0f"); + + SecureBlob blob; + SecureBlob::HexStringToSecureBlob(hex_string, &blob); + + EXPECT_EQ(blob.size(), 16u); + EXPECT_EQ(blob[0], 0x11); + EXPECT_EQ(blob[1], 0x22); + EXPECT_EQ(blob[2], 0x33); + EXPECT_EQ(blob[3], 0x44); + EXPECT_EQ(blob[4], 0x55); + EXPECT_EQ(blob[5], 0x66); + EXPECT_EQ(blob[6], 0x77); + EXPECT_EQ(blob[7], 0x88); + EXPECT_EQ(blob[8], 0x99); + EXPECT_EQ(blob[9], 0xaa); + EXPECT_EQ(blob[10], 0xbb); + EXPECT_EQ(blob[11], 0xcc); + EXPECT_EQ(blob[12], 0xdd); + EXPECT_EQ(blob[13], 0xee); + EXPECT_EQ(blob[14], 0xff); + EXPECT_EQ(blob[15], 0x0f); +} + } // namespace brillo diff --git a/brillo/streams/fake_stream_unittest.cc b/brillo/streams/fake_stream_unittest.cc index df8b223..2404514 100644 --- a/brillo/streams/fake_stream_unittest.cc +++ b/brillo/streams/fake_stream_unittest.cc @@ -236,23 +236,19 @@ TEST_F(FakeStreamTest, WaitForDataRead) { EXPECT_CALL(mock_loop_, PostDelayedTask(_, _, zero_delay)).Times(2); int call_count = 0; - auto callback = [](int* call_count, Stream::AccessMode mode) { + auto callback = base::Bind([](int* call_count, Stream::AccessMode mode) { (*call_count)++; EXPECT_EQ(Stream::AccessMode::READ, mode); - }; + }, &call_count); - EXPECT_TRUE( - stream_->WaitForData(Stream::AccessMode::READ, - base::Bind(callback, base::Unretained(&call_count)), - nullptr)); + EXPECT_TRUE(stream_->WaitForData(Stream::AccessMode::READ, + callback, nullptr)); mock_loop_.Run(); EXPECT_EQ(1, call_count); stream_->AddReadPacketString({}, "foobar"); - EXPECT_TRUE( - stream_->WaitForData(Stream::AccessMode::READ, - base::Bind(callback, base::Unretained(&call_count)), - nullptr)); + EXPECT_TRUE(stream_->WaitForData(Stream::AccessMode::READ, + callback, nullptr)); mock_loop_.Run(); EXPECT_EQ(2, call_count); @@ -261,10 +257,8 @@ TEST_F(FakeStreamTest, WaitForDataRead) { auto one_sec_delay = base::TimeDelta::FromSeconds(1); stream_->AddReadPacketString(one_sec_delay, "baz"); EXPECT_CALL(mock_loop_, PostDelayedTask(_, _, one_sec_delay)).Times(1); - EXPECT_TRUE( - stream_->WaitForData(Stream::AccessMode::READ, - base::Bind(callback, base::Unretained(&call_count)), - nullptr)); + EXPECT_TRUE(stream_->WaitForData(Stream::AccessMode::READ, + callback, nullptr)); mock_loop_.Run(); EXPECT_EQ(3, call_count); } @@ -401,23 +395,19 @@ TEST_F(FakeStreamTest, WaitForDataWrite) { EXPECT_CALL(mock_loop_, PostDelayedTask(_, _, zero_delay)).Times(2); int call_count = 0; - auto callback = [](int* call_count, Stream::AccessMode mode) { + auto callback = base::Bind([](int* call_count, Stream::AccessMode mode) { (*call_count)++; EXPECT_EQ(Stream::AccessMode::WRITE, mode); - }; + }, &call_count); - EXPECT_TRUE( - stream_->WaitForData(Stream::AccessMode::WRITE, - base::Bind(callback, base::Unretained(&call_count)), - nullptr)); + EXPECT_TRUE(stream_->WaitForData(Stream::AccessMode::WRITE, + callback, nullptr)); mock_loop_.Run(); EXPECT_EQ(1, call_count); stream_->ExpectWritePacketString({}, "foobar"); - EXPECT_TRUE( - stream_->WaitForData(Stream::AccessMode::WRITE, - base::Bind(callback, base::Unretained(&call_count)), - nullptr)); + EXPECT_TRUE(stream_->WaitForData(Stream::AccessMode::WRITE, + callback, nullptr)); mock_loop_.Run(); EXPECT_EQ(2, call_count); @@ -426,10 +416,8 @@ TEST_F(FakeStreamTest, WaitForDataWrite) { auto one_sec_delay = base::TimeDelta::FromSeconds(1); stream_->ExpectWritePacketString(one_sec_delay, "baz"); EXPECT_CALL(mock_loop_, PostDelayedTask(_, _, one_sec_delay)).Times(1); - EXPECT_TRUE( - stream_->WaitForData(Stream::AccessMode::WRITE, - base::Bind(callback, base::Unretained(&call_count)), - nullptr)); + EXPECT_TRUE(stream_->WaitForData(Stream::AccessMode::WRITE, + callback, nullptr)); mock_loop_.Run(); EXPECT_EQ(3, call_count); } diff --git a/brillo/streams/file_stream_unittest.cc b/brillo/streams/file_stream_unittest.cc index aa1ce43..210725e 100644 --- a/brillo/streams/file_stream_unittest.cc +++ b/brillo/streams/file_stream_unittest.cc @@ -116,6 +116,15 @@ void TestCreateFile(Stream* stream) { EXPECT_EQ(-1, ReadByte(stream)); } +// Helper functions for base::Bind. +void SetSizeT(size_t* target, size_t source) { + *target = source; +} + +void SetToTrue(bool* target, const Error* /* error */) { + *target = true; +} + } // anonymous namespace // A mock file descriptor wrapper to test low-level file API used by FileStream. @@ -371,24 +380,16 @@ TEST_F(FileStreamTest, Seek_Fail) { TEST_F(FileStreamTest, ReadAsync) { size_t read_size = 0; bool failed = false; - auto success_callback = [](size_t* read_size, size_t size) { - *read_size = size; - }; - auto error_callback = [](bool* failed, const Error* /* error */) { - *failed = true; - }; FileStream::FileDescriptorInterface::DataCallback data_callback; EXPECT_CALL(fd_mock(), Read(test_read_buffer_, 100)) .WillOnce(ReturnWouldBlock()); EXPECT_CALL(fd_mock(), WaitForData(Stream::AccessMode::READ, _, _)) .WillOnce(DoAll(SaveArg<1>(&data_callback), Return(true))); - EXPECT_TRUE(stream_->ReadAsync( - test_read_buffer_, - 100, - base::Bind(success_callback, base::Unretained(&read_size)), - base::Bind(error_callback, base::Unretained(&failed)), - nullptr)); + EXPECT_TRUE(stream_->ReadAsync(test_read_buffer_, 100, + base::Bind(&SetSizeT, &read_size), + base::Bind(&SetToTrue, &failed), + nullptr)); EXPECT_EQ(0u, read_size); EXPECT_FALSE(failed); @@ -516,24 +517,16 @@ TEST_F(FileStreamTest, ReadAllBlocking_Fail) { TEST_F(FileStreamTest, WriteAsync) { size_t write_size = 0; bool failed = false; - auto success_callback = [](size_t* write_size, size_t size) { - *write_size = size; - }; - auto error_callback = [](bool* failed, const Error* /* error */) { - *failed = true; - }; FileStream::FileDescriptorInterface::DataCallback data_callback; EXPECT_CALL(fd_mock(), Write(test_write_buffer_, 100)) .WillOnce(ReturnWouldBlock()); EXPECT_CALL(fd_mock(), WaitForData(Stream::AccessMode::WRITE, _, _)) .WillOnce(DoAll(SaveArg<1>(&data_callback), Return(true))); - EXPECT_TRUE(stream_->WriteAsync( - test_write_buffer_, - 100, - base::Bind(success_callback, base::Unretained(&write_size)), - base::Bind(error_callback, base::Unretained(&failed)), - nullptr)); + EXPECT_TRUE(stream_->WriteAsync(test_write_buffer_, 100, + base::Bind(&SetSizeT, &write_size), + base::Bind(&SetToTrue, &failed), + nullptr)); EXPECT_EQ(0u, write_size); EXPECT_FALSE(failed); @@ -1051,10 +1044,6 @@ TEST_F(FileStreamTest, FromFileDescriptor_ReadAsync) { *succeeded = true; }; - auto error_callback = [](bool* failed, const Error* /* error */) { - *failed = true; - }; - auto write_data_callback = [](int write_fd) { std::string data{"abracadabra"}; EXPECT_TRUE(base::WriteFileDescriptor(write_fd, data.data(), data.size())); @@ -1070,14 +1059,9 @@ TEST_F(FileStreamTest, FromFileDescriptor_ReadAsync) { base::Bind(write_data_callback, fds[1]), base::TimeDelta::FromMilliseconds(10)); - EXPECT_TRUE( - stream->ReadAsync(buffer, - 100, - base::Bind(success_callback, - base::Unretained(&succeeded), - base::Unretained(buffer)), - base::Bind(error_callback, base::Unretained(&failed)), - nullptr)); + EXPECT_TRUE(stream->ReadAsync( + buffer, 100, base::Bind(success_callback, &succeeded, buffer), + base::Bind(&SetToTrue, &failed), nullptr)); auto end_condition = [](bool* failed, bool* succeeded) { return *failed || *succeeded; @@ -1116,17 +1100,13 @@ TEST_F(FileStreamTest, FromFileDescriptor_WriteAsync) { *succeeded = true; }; - auto error_callback = [](bool* failed, const Error* /* error */) { - *failed = true; - }; - StreamPtr stream = FileStream::FromFileDescriptor(fds[1], true, nullptr); EXPECT_TRUE(stream->WriteAsync( data.data(), data.size(), base::Bind(success_callback, base::Unretained(&succeeded), data, fds[0]), - base::Bind(error_callback, base::Unretained(&failed)), + base::Bind(&SetToTrue, &failed), nullptr)); auto end_condition = [](bool* failed, bool* succeeded) { diff --git a/brillo/streams/stream_unittest.cc b/brillo/streams/stream_unittest.cc index 625b569..c341cde 100644 --- a/brillo/streams/stream_unittest.cc +++ b/brillo/streams/stream_unittest.cc @@ -21,6 +21,15 @@ using testing::SaveArg; using testing::SetArgPointee; using testing::_; +namespace { + +// Helper function for base::Bind. +void SetToTrue(bool* target, const brillo::Error* /* error */) { + *target = true; +} + +} // namespace + namespace brillo { using AccessMode = Stream::AccessMode; @@ -100,13 +109,12 @@ TEST(Stream, ReadAsync) { size_t read_size = 0; bool succeeded = false; bool failed = false; - auto success_callback = [](size_t* read_size, bool* succeeded,size_t size) { + auto success_callback = base::Bind( + [](size_t* read_size, bool* succeeded, size_t size) { *read_size = size; *succeeded = true; - }; - auto error_callback = [](bool* failed, const Error* /* error */) { - *failed = true; - }; + }, &read_size, &succeeded); + auto error_callback = base::Bind(&SetToTrue, &failed); MockStreamImpl stream_mock; base::Callback<void(AccessMode)> data_callback; @@ -119,14 +127,8 @@ TEST(Stream, ReadAsync) { DoAll(SetArgPointee<2>(0), SetArgPointee<3>(false), Return(true))); EXPECT_CALL(stream_mock, WaitForData(AccessMode::READ, _, _)) .WillOnce(DoAll(SaveArg<1>(&data_callback), Return(true))); - EXPECT_TRUE(stream_mock.ReadAsync( - buf, - sizeof(buf), - base::Bind(success_callback, - base::Unretained(&read_size), - base::Unretained(&succeeded)), - base::Bind(error_callback, base::Unretained(&failed)), - nullptr)); + EXPECT_TRUE(stream_mock.ReadAsync(buf, sizeof(buf), success_callback, + error_callback, nullptr)); EXPECT_EQ(0u, read_size); EXPECT_FALSE(succeeded); EXPECT_FALSE(failed); @@ -134,14 +136,8 @@ TEST(Stream, ReadAsync) { // Since the previous call is waiting for the data to be available, we can't // schedule another read. ErrorPtr error; - EXPECT_FALSE(stream_mock.ReadAsync( - buf, - sizeof(buf), - base::Bind(success_callback, - base::Unretained(&read_size), - base::Unretained(&succeeded)), - base::Bind(error_callback, base::Unretained(&failed)), - &error)); + EXPECT_FALSE(stream_mock.ReadAsync(buf, sizeof(buf), success_callback, + error_callback, &error)); EXPECT_EQ(errors::stream::kDomain, error->GetDomain()); EXPECT_EQ(errors::stream::kOperationNotSupported, error->GetCode()); EXPECT_EQ("Another asynchronous operation is still pending", @@ -161,12 +157,10 @@ TEST(Stream, ReadAsync) { TEST(Stream, ReadAsync_DontWaitForData) { bool succeeded = false; bool failed = false; - auto success_callback = [](bool* succeeded, size_t /* size */) { + auto success_callback = base::Bind([](bool* succeeded, size_t /* size */) { *succeeded = true; - }; - auto error_callback = [](bool* failed, const Error* /* error */) { - *failed = true; - }; + }, &succeeded); + auto error_callback = base::Bind(&SetToTrue, &failed); MockStreamImpl stream_mock; char buf[10]; @@ -177,12 +171,8 @@ TEST(Stream, ReadAsync_DontWaitForData) { .WillOnce( DoAll(SetArgPointee<2>(5), SetArgPointee<3>(false), Return(true))); EXPECT_CALL(stream_mock, WaitForData(_, _, _)).Times(0); - EXPECT_TRUE(stream_mock.ReadAsync( - buf, - sizeof(buf), - base::Bind(success_callback, base::Unretained(&succeeded)), - base::Bind(error_callback, base::Unretained(&failed)), - nullptr)); + EXPECT_TRUE(stream_mock.ReadAsync(buf, sizeof(buf), success_callback, + error_callback, nullptr)); // Even if ReadNonBlocking() returned some data without waiting, the // |success_callback| should not run yet. EXPECT_TRUE(fake_loop_.PendingTasks()); @@ -192,12 +182,8 @@ TEST(Stream, ReadAsync_DontWaitForData) { // Since the previous callback is still waiting in the main loop, we can't // schedule another read yet. ErrorPtr error; - EXPECT_FALSE(stream_mock.ReadAsync( - buf, - sizeof(buf), - base::Bind(success_callback, base::Unretained(&succeeded)), - base::Bind(error_callback, base::Unretained(&failed)), - &error)); + EXPECT_FALSE(stream_mock.ReadAsync(buf, sizeof(buf), success_callback, + error_callback, &error)); EXPECT_EQ(errors::stream::kDomain, error->GetDomain()); EXPECT_EQ(errors::stream::kOperationNotSupported, error->GetCode()); EXPECT_EQ("Another asynchronous operation is still pending", @@ -211,10 +197,9 @@ TEST(Stream, ReadAsync_DontWaitForData) { TEST(Stream, ReadAllAsync) { bool succeeded = false; bool failed = false; - auto success_callback = [](bool* succeeded) { *succeeded = true; }; - auto error_callback = [](bool* failed, const Error* /* error */) { - *failed = true; - }; + auto success_callback = base::Bind([](bool* succeeded) { *succeeded = true; }, + &succeeded); + auto error_callback = base::Bind(&SetToTrue, &failed); MockStreamImpl stream_mock; base::Callback<void(AccessMode)> data_callback; @@ -227,12 +212,8 @@ TEST(Stream, ReadAllAsync) { DoAll(SetArgPointee<2>(0), SetArgPointee<3>(false), Return(true))); EXPECT_CALL(stream_mock, WaitForData(AccessMode::READ, _, _)) .WillOnce(DoAll(SaveArg<1>(&data_callback), Return(true))); - EXPECT_TRUE(stream_mock.ReadAllAsync( - buf, - sizeof(buf), - base::Bind(success_callback, base::Unretained(&succeeded)), - base::Bind(error_callback, base::Unretained(&failed)), - nullptr)); + EXPECT_TRUE(stream_mock.ReadAllAsync(buf, sizeof(buf), success_callback, + error_callback, nullptr)); EXPECT_FALSE(succeeded); EXPECT_FALSE(failed); testing::Mock::VerifyAndClearExpectations(&stream_mock); @@ -265,12 +246,13 @@ TEST(Stream, ReadAllAsync) { TEST(Stream, ReadAllAsync_EOS) { bool succeeded = false; bool failed = false; - auto success_callback = [](bool* succeeded) { *succeeded = true; }; - auto error_callback = [](bool* failed, const Error* error) { + auto success_callback = base::Bind([](bool* succeeded) { *succeeded = true; }, + &succeeded); + auto error_callback = base::Bind([](bool* failed, const Error* error) { ASSERT_EQ(errors::stream::kDomain, error->GetDomain()); ASSERT_EQ(errors::stream::kPartialData, error->GetCode()); *failed = true; - }; + }, &failed); MockStreamImpl stream_mock; base::Callback<void(AccessMode)> data_callback; @@ -281,12 +263,8 @@ TEST(Stream, ReadAllAsync_EOS) { DoAll(SetArgPointee<2>(0), SetArgPointee<3>(false), Return(true))); EXPECT_CALL(stream_mock, WaitForData(AccessMode::READ, _, _)) .WillOnce(DoAll(SaveArg<1>(&data_callback), Return(true))); - EXPECT_TRUE(stream_mock.ReadAllAsync( - buf, - sizeof(buf), - base::Bind(success_callback, base::Unretained(&succeeded)), - base::Bind(error_callback, base::Unretained(&failed)), - nullptr)); + EXPECT_TRUE(stream_mock.ReadAllAsync(buf, sizeof(buf), success_callback, + error_callback, nullptr)); // ReadAsyncAll() should finish and fail once ReadNonBlocking() returns an // end-of-stream condition. @@ -379,12 +357,10 @@ TEST(Stream, ReadAllBlocking) { TEST(Stream, WriteAsync) { size_t write_size = 0; bool failed = false; - auto success_callback = [](size_t* write_size, size_t size) { + auto success_callback = base::Bind([](size_t* write_size, size_t size) { *write_size = size; - }; - auto error_callback = [](bool* failed, const Error* /* error */) { - *failed = true; - }; + }, &write_size); + auto error_callback = base::Bind(&SetToTrue, &failed); MockStreamImpl stream_mock; InSequence s; @@ -397,22 +373,14 @@ TEST(Stream, WriteAsync) { .WillOnce(DoAll(SetArgPointee<2>(0), Return(true))); EXPECT_CALL(stream_mock, WaitForData(AccessMode::WRITE, _, _)) .WillOnce(DoAll(SaveArg<1>(&data_callback), Return(true))); - EXPECT_TRUE(stream_mock.WriteAsync( - buf, - sizeof(buf), - base::Bind(success_callback, base::Unretained(&write_size)), - base::Bind(error_callback, base::Unretained(&failed)), - nullptr)); + EXPECT_TRUE(stream_mock.WriteAsync(buf, sizeof(buf), success_callback, + error_callback, nullptr)); EXPECT_EQ(0u, write_size); EXPECT_FALSE(failed); ErrorPtr error; - EXPECT_FALSE(stream_mock.WriteAsync( - buf, - sizeof(buf), - base::Bind(success_callback, base::Unretained(&write_size)), - base::Bind(error_callback, base::Unretained(&failed)), - &error)); + EXPECT_FALSE(stream_mock.WriteAsync(buf, sizeof(buf), success_callback, + error_callback, &error)); EXPECT_EQ(errors::stream::kDomain, error->GetDomain()); EXPECT_EQ(errors::stream::kOperationNotSupported, error->GetCode()); EXPECT_EQ("Another asynchronous operation is still pending", @@ -428,10 +396,9 @@ TEST(Stream, WriteAsync) { TEST(Stream, WriteAllAsync) { bool succeeded = false; bool failed = false; - auto success_callback = [](bool* succeeded) { *succeeded = true; }; - auto error_callback = [](bool* failed, const Error* /* error */) { - *failed = true; - }; + auto success_callback = base::Bind([](bool* succeeded) { *succeeded = true; }, + &succeeded); + auto error_callback = base::Bind(&SetToTrue, &failed); MockStreamImpl stream_mock; base::Callback<void(AccessMode)> data_callback; @@ -441,12 +408,8 @@ TEST(Stream, WriteAllAsync) { .WillOnce(DoAll(SetArgPointee<2>(0), Return(true))); EXPECT_CALL(stream_mock, WaitForData(AccessMode::WRITE, _, _)) .WillOnce(DoAll(SaveArg<1>(&data_callback), Return(true))); - EXPECT_TRUE(stream_mock.WriteAllAsync( - buf, - sizeof(buf), - base::Bind(success_callback, base::Unretained(&succeeded)), - base::Bind(error_callback, base::Unretained(&failed)), - nullptr)); + EXPECT_TRUE(stream_mock.WriteAllAsync(buf, sizeof(buf), success_callback, + error_callback, nullptr)); testing::Mock::VerifyAndClearExpectations(&stream_mock); EXPECT_FALSE(succeeded); EXPECT_FALSE(failed); diff --git a/brillo/syslog_logging.cc b/brillo/syslog_logging.cc index 40b064a..2b35280 100644 --- a/brillo/syslog_logging.cc +++ b/brillo/syslog_logging.cc @@ -5,6 +5,7 @@ #include "brillo/syslog_logging.h" #include <syslog.h> +#include <unistd.h> #include <string> @@ -78,6 +79,8 @@ namespace brillo { void SetLogFlags(int log_flags) { s_log_to_syslog = (log_flags & kLogToSyslog) != 0; s_log_to_stderr = (log_flags & kLogToStderr) != 0; + if ((log_flags & kLogToStderrIfTty) && isatty(0)) + s_log_to_stderr = true; s_log_header = (log_flags & kLogHeader) != 0; } int GetLogFlags() { diff --git a/brillo/syslog_logging.h b/brillo/syslog_logging.h index b054259..04b3253 100644 --- a/brillo/syslog_logging.h +++ b/brillo/syslog_logging.h @@ -12,9 +12,14 @@ namespace brillo { enum InitFlags { + // Always log to syslog. kLogToSyslog = 1, + // Always log to stderr. kLogToStderr = 2, + // Include message header in log lines. kLogHeader = 4, + // Log to stderr if stdin is a tty (e.g. command line). + kLogToStderrIfTty = 8, }; // Initialize logging subsystem. |init_flags| is a bitfield, with bits defined diff --git a/brillo/value_conversion.cc b/brillo/value_conversion.cc index 8aab94c..dfe8797 100644 --- a/brillo/value_conversion.cc +++ b/brillo/value_conversion.cc @@ -24,7 +24,7 @@ bool FromValue(const base::Value& in_value, const base::ListValue* list = nullptr; if (!in_value.GetAsList(&list)) return false; - out_value->reset(list->DeepCopy()); + *out_value = list->CreateDeepCopy(); return true; } @@ -33,28 +33,28 @@ bool FromValue(const base::Value& in_value, const base::DictionaryValue* dict = nullptr; if (!in_value.GetAsDictionary(&dict)) return false; - out_value->reset(dict->DeepCopy()); + *out_value = dict->CreateDeepCopy(); return true; } std::unique_ptr<base::Value> ToValue(int value) { - return std::unique_ptr<base::Value>(new base::Value(value)); + return std::make_unique<base::Value>(value); } std::unique_ptr<base::Value> ToValue(bool value) { - return std::unique_ptr<base::Value>(new base::Value(value)); + return std::make_unique<base::Value>(value); } std::unique_ptr<base::Value> ToValue(double value) { - return std::unique_ptr<base::Value>(new base::Value(value)); + return std::make_unique<base::Value>(value); } std::unique_ptr<base::Value> ToValue(const char* value) { - return std::unique_ptr<base::Value>(new base::Value(value)); + return std::make_unique<base::Value>(value); } std::unique_ptr<base::Value> ToValue(const std::string& value) { - return std::unique_ptr<base::Value>(new base::Value(value)); + return std::make_unique<base::Value>(value); } } // namespace brillo diff --git a/install_attributes/libinstallattributes.cc b/install_attributes/libinstallattributes.cc new file mode 100644 index 0000000..52f9f70 --- /dev/null +++ b/install_attributes/libinstallattributes.cc @@ -0,0 +1,82 @@ +// Copyright 2016 The Chromium OS 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 "libinstallattributes.h" + +#include <base/files/file_util.h> +#include <base/logging.h> + +#include "bindings/install_attributes.pb.h" + +namespace { + +// Written by cryptohome or by lockbox-cache after signature verification and +// thus guaranteed to be unadulterated. +const char kInstallAttributesPath[] = "/run/lockbox/install_attributes.pb"; + +} // namespace + +// The source of truth for these constants is Chromium +// //chrome/browser/chromeos/settings/install_attributes.cc. +const char InstallAttributesReader::kAttrMode[] = "enterprise.mode"; +const char InstallAttributesReader::kDeviceModeConsumer[] = "consumer"; +const char InstallAttributesReader::kDeviceModeEnterprise[] = "enterprise"; +const char InstallAttributesReader::kDeviceModeEnterpriseAD[] = "enterprise_ad"; +const char InstallAttributesReader::kDeviceModeLegacyRetail[] = "kiosk"; +const char InstallAttributesReader::kDeviceModeConsumerKiosk[] = + "consumer_kiosk"; + +InstallAttributesReader::InstallAttributesReader() + : install_attributes_path_(kInstallAttributesPath) { +} + +InstallAttributesReader::~InstallAttributesReader() { +} + +const std::string& InstallAttributesReader::GetAttribute( + const std::string& key) { + // By its very nature of immutable attributes, once read successfully the + // attributes can never change and thus never need reloading. + if (!initialized_) { + TryToLoad(); + } + + const auto entry = attributes_.find(key); + if (entry == attributes_.end()) { + return empty_string_; + } + return entry->second; +} + +bool InstallAttributesReader::IsLocked() { + if (!initialized_) { + TryToLoad(); + } + return initialized_; +} + +void InstallAttributesReader::TryToLoad() { + std::string contents; + if (!base::ReadFileToString(install_attributes_path_, &contents)) { + // May fail during OOBE or early in the boot process. + return; + } + + // Parse errors are unrecoverable (lockbox does atomic write), thus mark as + // inititialized already before checking for parse errors. + initialized_ = true; + + cryptohome::SerializedInstallAttributes install_attributes; + if (!install_attributes.ParseFromString(contents)) { + LOG(ERROR) << "Can't parse install attributes."; + return; + } + + for (int i = 0; i < install_attributes.attributes_size(); ++i) { + const cryptohome::SerializedInstallAttributes_Attribute& attribute = + install_attributes.attributes(i); + // Cast value to C string and back to remove trailing zero. + attributes_[attribute.name()] = std::string(attribute.value().c_str()); + } +} diff --git a/install_attributes/libinstallattributes.h b/install_attributes/libinstallattributes.h new file mode 100644 index 0000000..b947156 --- /dev/null +++ b/install_attributes/libinstallattributes.h @@ -0,0 +1,66 @@ +// Copyright 2016 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef LIBBRILLO_INSTALL_ATTRIBUTES_LIBINSTALLATTRIBUTES_H_ +#define LIBBRILLO_INSTALL_ATTRIBUTES_LIBINSTALLATTRIBUTES_H_ + +#include <map> +#include <string> + +#include <base/files/file_path.h> +#include <brillo/brillo_export.h> + +// Simple caching reader for the (verified) install attributes, a TPM-backed +// write once read many store. Install attributes may be written exactly once +// by a single, atomic write-and-lock operation encompassing zero or more +// attributes. Once locked, install attributes cannot be re-written unless TPM +// is reset (eg. by powerwashing the device). +class BRILLO_EXPORT InstallAttributesReader { + public: + static const char kAttrMode[]; + + // Constants for the possible device modes. + static const char kDeviceModeConsumer[]; + static const char kDeviceModeEnterprise[]; + static const char kDeviceModeEnterpriseAD[]; + static const char kDeviceModeLegacyRetail[]; + static const char kDeviceModeConsumerKiosk[]; + + InstallAttributesReader(); + virtual ~InstallAttributesReader(); + + // Try to load install attributes (unless cached already) and return the + // attribute for |key| or an empty string in case |key| doesn't exist or in + // case install attributes couldn't (yet) be loaded. The latter is expected + // during OOBE (install attributes haven't yet been finalized) or early in the + // boot sequence (install attributes haven't yet been verified). + const std::string& GetAttribute(const std::string& key); + + // Try to load install attributes (unless cached already) and return whether + // they have yet been written-and-locked. + bool IsLocked(); + + protected: + // Attributes cache. + std::map<std::string, std::string> attributes_; + + // Path to the *verified* install attributes file on disk. + base::FilePath install_attributes_path_; + + // Whether install attributes have been read successfully. Reading a file + // containing an empty attributes proto indicates consumer mode and counts as + // successful, too. + bool initialized_ = false; + +private: + // Try to load the verified install attributes from disk. This is expected to + // fail when install attributes haven't yet been finalized (OOBE) or verified + // (early in the boot sequence). + void TryToLoad(); + + // Empty string to return on error. + std::string empty_string_; +}; + +#endif // LIBBRILLO_LIBINSTALLATTRIBUTES_H_ diff --git a/install_attributes/mock_install_attributes_reader.cc b/install_attributes/mock_install_attributes_reader.cc new file mode 100644 index 0000000..da18988 --- /dev/null +++ b/install_attributes/mock_install_attributes_reader.cc @@ -0,0 +1,22 @@ +// Copyright 2016 The Chromium OS 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 "mock_install_attributes_reader.h" + +MockInstallAttributesReader::MockInstallAttributesReader( + const cryptohome::SerializedInstallAttributes& install_attributes) { + for (int i = 0; i < install_attributes.attributes_size(); ++i) { + const cryptohome::SerializedInstallAttributes_Attribute& attribute = + install_attributes.attributes(i); + // Cast value to C string and back to remove trailing zero. + attributes_[attribute.name()] = std::string(attribute.value().c_str()); + } + initialized_ = true; +} + +MockInstallAttributesReader::MockInstallAttributesReader( + const std::string& device_mode, bool initialized) { + attributes_[kAttrMode] = device_mode; + initialized_ = initialized; +} diff --git a/install_attributes/mock_install_attributes_reader.h b/install_attributes/mock_install_attributes_reader.h new file mode 100644 index 0000000..5ccee02 --- /dev/null +++ b/install_attributes/mock_install_attributes_reader.h @@ -0,0 +1,19 @@ +// Copyright 2016 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef LIBBRILLO_INSTALL_ATTRIBUTES_MOCK_INSTALL_ATTRIBUTES_READER_H_ +#define LIBBRILLO_INSTALL_ATTRIBUTES_MOCK_INSTALL_ATTRIBUTES_READER_H_ + +#include "libinstallattributes.h" + +#include "bindings/install_attributes.pb.h" + +class MockInstallAttributesReader : public InstallAttributesReader { + public: + explicit MockInstallAttributesReader( + const cryptohome::SerializedInstallAttributes& install_attributes); + MockInstallAttributesReader(const std::string& device_mode, bool initialized); +}; + +#endif // LIBBRILLO_INSTALL_ATTRIBUTES_MOCK_INSTALL_ATTRIBUTES_READER_H_ diff --git a/install_attributes/tests/consumer.pb b/install_attributes/tests/consumer.pb new file mode 100644 index 0000000..e19a122 --- /dev/null +++ b/install_attributes/tests/consumer.pb @@ -0,0 +1 @@ +
\ No newline at end of file diff --git a/install_attributes/tests/corrupt.pb b/install_attributes/tests/corrupt.pb new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/install_attributes/tests/corrupt.pb diff --git a/install_attributes/tests/libinstallattributes_unittest.cc b/install_attributes/tests/libinstallattributes_unittest.cc new file mode 100644 index 0000000..45ff827 --- /dev/null +++ b/install_attributes/tests/libinstallattributes_unittest.cc @@ -0,0 +1,83 @@ +// Copyright (c) 2012 The Chromium OS 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 "install_attributes/libinstallattributes.h" + +#include <string> + +#include <gtest/gtest.h> + +// Allows to override the install attributes path while preserving all the +// functionality of the original class. +class MockInstallAttributesReader : public InstallAttributesReader { + public: + void SetPath(const std::string& filename) { + install_attributes_path_ = base::FilePath(filename); + } + size_t GetAttributesCount() const { return attributes_.size(); } +}; + +TEST(InstallAttributesTest, ReadNonexistingAttributes) { + MockInstallAttributesReader reader; + reader.SetPath("non-existing.pb"); + ASSERT_FALSE(reader.IsLocked()); + ASSERT_EQ(0, reader.GetAttributesCount()); +} + +// corrupt.pb is an invalid proto. +TEST(InstallAttributesTest, ReadCorruptAttributes) { + MockInstallAttributesReader reader; + reader.SetPath("install_attributes/tests/corrupt.pb"); + ASSERT_TRUE(reader.IsLocked()); + ASSERT_EQ(0, reader.GetAttributesCount()); +} + +// consumer.pb is a valid proto containing no attributes. +TEST(InstallAttributesTest, ReadEmptyAttributes) { + MockInstallAttributesReader reader; + reader.SetPath("install_attributes/tests/consumer.pb"); + ASSERT_TRUE(reader.IsLocked()); + ASSERT_EQ(0, reader.GetAttributesCount()); +} + +// managed.pb is a valid proto containing the usual enterprise enrollment +// attributes. +TEST(InstallAttributesTest, ReadManagedAttributes) { + MockInstallAttributesReader reader; + reader.SetPath("install_attributes/tests/managed.pb"); + ASSERT_TRUE(reader.IsLocked()); + ASSERT_EQ(std::string(), reader.GetAttribute("non-existing")); + ASSERT_EQ("enterprise", reader.GetAttribute("enterprise.mode")); +} + +// Going from non-existing attributes file to existing attributes file must +// work, i.e. the non-existence of the attributes file must not be cached. +TEST(InstallAttributesTest, ProgressionFromNonExistingToManaged) { + MockInstallAttributesReader reader; + reader.SetPath("non-existing.pb"); + ASSERT_FALSE(reader.IsLocked()); + ASSERT_EQ(0, reader.GetAttributesCount()); + + reader.SetPath("install_attributes/tests/managed.pb"); + ASSERT_TRUE(reader.IsLocked()); + ASSERT_EQ("enterprise", reader.GetAttribute("enterprise.mode")); +} + +// Going from empty attributes file to non-empty attributes file must not work, +// i.e. the non-existence of the attributes must be cached. +TEST(InstallAttributesTest, NoProgressionFromEmptyToManaged) { + MockInstallAttributesReader reader; + reader.SetPath("install_attributes/tests/consumer.pb"); + ASSERT_TRUE(reader.IsLocked()); + ASSERT_EQ(0, reader.GetAttributesCount()); + + reader.SetPath("install_attributes/tests/managed.pb"); + ASSERT_TRUE(reader.IsLocked()); + ASSERT_EQ(std::string(), reader.GetAttribute("enterprise.mode")); +} + +int main(int argc, char* argv[]) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/install_attributes/tests/managed.pb b/install_attributes/tests/managed.pb Binary files differnew file mode 100644 index 0000000..4060a49 --- /dev/null +++ b/install_attributes/tests/managed.pb diff --git a/libbrillo.gyp b/libbrillo.gyp index 487e99d..5a2bbe4 100644 --- a/libbrillo.gyp +++ b/libbrillo.gyp @@ -1,6 +1,7 @@ { 'includes': [ 'libbrillo-395517.gypi', + 'libinstallattributes.gypi', 'libpolicy.gypi', ] } diff --git a/libbrillo.gypi b/libbrillo.gypi index 1788775..05d95e6 100644 --- a/libbrillo.gypi +++ b/libbrillo.gypi @@ -4,7 +4,6 @@ 'deps': [ 'libchrome-<(libbase_ver)' ], - 'USE_dbus%': '1', }, 'include_dirs': [ '../libbrillo', @@ -24,6 +23,7 @@ 'libbrillo-http-<(libbase_ver)', 'libbrillo-minijail-<(libbase_ver)', 'libbrillo-streams-<(libbase_ver)', + 'libinstallattributes-<(libbase_ver)', 'libpolicy-<(libbase_ver)', ], 'direct_dependent_settings': { @@ -38,7 +38,13 @@ 'type': 'shared_library', 'variables': { 'exported_deps': [ - 'dbus-1', + ], + 'conditions': [ + ['USE_dbus == 1', { + 'exported_deps': [ + 'dbus-1', + ], + }], ], 'deps': ['<@(exported_deps)'], }, @@ -50,30 +56,16 @@ }, }, 'libraries': ['-lmodp_b64'], - #TODO(deymo): Split DBus code from libbrillo-core the same way is split in - # the Android.mk, based on the <(USE_dbus) variable. 'sources': [ - 'brillo/any.cc', 'brillo/asynchronous_signal_handler.cc', 'brillo/backoff_entry.cc', - 'brillo/daemons/dbus_daemon.cc', 'brillo/daemons/daemon.cc', 'brillo/data_encoding.cc', - 'brillo/dbus/async_event_sequencer.cc', - 'brillo/dbus/data_serialization.cc', - 'brillo/dbus/dbus_connection.cc', - 'brillo/dbus/dbus_method_invoker.cc', - 'brillo/dbus/dbus_method_response.cc', - 'brillo/dbus/dbus_object.cc', - 'brillo/dbus/dbus_service_watcher.cc', - 'brillo/dbus/dbus_signal.cc', - 'brillo/dbus/exported_object_manager.cc', - 'brillo/dbus/exported_property_set.cc', - 'brillo/dbus/utils.cc', 'brillo/errors/error.cc', 'brillo/errors/error_codes.cc', 'brillo/file_utils.cc', 'brillo/flag_helper.cc', + 'brillo/imageloader/manifest.cc', 'brillo/key_value_store.cc', 'brillo/message_loops/base_message_loop.cc', 'brillo/message_loops/message_loop.cc', @@ -91,6 +83,25 @@ 'brillo/userdb_utils.cc', 'brillo/value_conversion.cc', ], + 'conditions': [ + ['USE_dbus == 1', { + 'sources': [ + 'brillo/any.cc', + 'brillo/daemons/dbus_daemon.cc', + 'brillo/dbus/async_event_sequencer.cc', + 'brillo/dbus/data_serialization.cc', + 'brillo/dbus/dbus_connection.cc', + 'brillo/dbus/dbus_method_invoker.cc', + 'brillo/dbus/dbus_method_response.cc', + 'brillo/dbus/dbus_object.cc', + 'brillo/dbus/dbus_service_watcher.cc', + 'brillo/dbus/dbus_signal.cc', + 'brillo/dbus/exported_object_manager.cc', + 'brillo/dbus/exported_property_set.cc', + 'brillo/dbus/utils.cc', + ], + }], + ], }, { 'target_name': 'libbrillo-http-<(libbase_ver)', @@ -121,6 +132,13 @@ 'brillo/http/http_transport_curl.cc', 'brillo/http/http_utils.cc', ], + 'conditions': [ + ['USE_dbus == 1', { + 'sources': [ + 'brillo/http/http_proxy.cc', + ], + }], + ], }, { 'target_name': 'libbrillo-streams-<(libbase_ver)', @@ -213,9 +231,34 @@ ], }, { + 'target_name': 'libinstallattributes-<(libbase_ver)', + 'type': 'shared_library', + 'dependencies': [ + 'libinstallattributes-includes', + '../common-mk/external_dependencies.gyp:install_attributes-proto', + ], + 'variables': { + 'exported_deps': [ + 'protobuf-lite', + ], + 'deps': ['<@(exported_deps)'], + }, + 'all_dependent_settings': { + 'variables': { + 'deps': [ + '<@(exported_deps)', + ], + }, + }, + 'sources': [ + 'install_attributes/libinstallattributes.cc', + ], + }, + { 'target_name': 'libpolicy-<(libbase_ver)', 'type': 'shared_library', 'dependencies': [ + 'libinstallattributes-<(libbase_ver)', 'libpolicy-includes', '../common-mk/external_dependencies.gyp:policy-protos', ], @@ -248,15 +291,21 @@ 'target_name': 'libbrillo-glib-<(libbase_ver)', 'type': 'shared_library', 'dependencies': [ - 'libbrillo-<(libbase_ver)', + 'libbrillo-<(libbase_ver)', ], 'variables': { 'exported_deps': [ - 'dbus-1', - 'dbus-glib-1', 'glib-2.0', 'gobject-2.0', ], + 'conditions': [ + ['USE_dbus == 1', { + 'exported_deps': [ + 'dbus-1', + 'dbus-glib-1', + ], + }], + ], 'deps': ['<@(exported_deps)'], }, 'cflags': [ @@ -270,12 +319,15 @@ ], }, }, - 'sources': [ - 'brillo/glib/abstract_dbus_service.cc', - 'brillo/glib/dbus.cc', - 'brillo/message_loops/glib_message_loop.cc', - ], 'includes': ['../common-mk/deps.gypi'], + 'conditions': [ + ['USE_dbus == 1', { + 'sources': [ + 'brillo/glib/abstract_dbus_service.cc', + 'brillo/glib/dbus.cc', + ], + }], + ], }, ], 'conditions': [ @@ -286,8 +338,8 @@ 'type': 'executable', 'dependencies': [ 'libbrillo-<(libbase_ver)', - 'libbrillo-test-<(libbase_ver)', 'libbrillo-glib-<(libbase_ver)', + 'libbrillo-test-<(libbase_ver)', ], 'variables': { 'deps': [ @@ -316,20 +368,10 @@ }], ], 'sources': [ - 'brillo/any_unittest.cc', - 'brillo/any_internal_impl_unittest.cc', 'brillo/asynchronous_signal_handler_unittest.cc', 'brillo/backoff_entry_unittest.cc', 'brillo/data_encoding_unittest.cc', - 'brillo/dbus/async_event_sequencer_unittest.cc', - 'brillo/dbus/data_serialization_unittest.cc', - 'brillo/dbus/dbus_method_invoker_unittest.cc', - 'brillo/dbus/dbus_object_unittest.cc', - 'brillo/dbus/dbus_param_reader_unittest.cc', - 'brillo/dbus/dbus_param_writer_unittest.cc', - 'brillo/dbus/dbus_signal_handler_unittest.cc', - 'brillo/dbus/exported_object_manager_unittest.cc', - 'brillo/dbus/exported_property_set_unittest.cc', + 'brillo/enum_flags_unittest.cc', 'brillo/errors/error_codes_unittest.cc', 'brillo/errors/error_unittest.cc', 'brillo/file_utils_unittest.cc', @@ -340,11 +382,11 @@ 'brillo/http/http_request_unittest.cc', 'brillo/http/http_transport_curl_unittest.cc', 'brillo/http/http_utils_unittest.cc', + 'brillo/imageloader/manifest_unittest.cc', 'brillo/key_value_store_unittest.cc', 'brillo/map_utils_unittest.cc', 'brillo/message_loops/base_message_loop_unittest.cc', 'brillo/message_loops/fake_message_loop_unittest.cc', - 'brillo/message_loops/glib_message_loop_unittest.cc', 'brillo/message_loops/message_loop_unittest.cc', 'brillo/mime_utils_unittest.cc', 'brillo/osrelease_reader_unittest.cc', @@ -360,21 +402,58 @@ 'brillo/streams/stream_unittest.cc', 'brillo/streams/stream_utils_unittest.cc', 'brillo/strings/string_utils_unittest.cc', - 'brillo/type_name_undecorate_unittest.cc', 'brillo/unittest_utils.cc', 'brillo/url_utils_unittest.cc', - 'brillo/variant_dictionary_unittest.cc', 'brillo/value_conversion_unittest.cc', 'testrunner.cc', - '<(proto_in_dir)/test.proto', + ], + 'conditions': [ + ['USE_dbus == 1', { + 'sources': [ + 'brillo/any_unittest.cc', + 'brillo/any_internal_impl_unittest.cc', + 'brillo/dbus/async_event_sequencer_unittest.cc', + 'brillo/dbus/data_serialization_unittest.cc', + 'brillo/dbus/dbus_method_invoker_unittest.cc', + 'brillo/dbus/dbus_object_unittest.cc', + 'brillo/dbus/dbus_param_reader_unittest.cc', + 'brillo/dbus/dbus_param_writer_unittest.cc', + 'brillo/dbus/dbus_signal_handler_unittest.cc', + 'brillo/dbus/exported_object_manager_unittest.cc', + 'brillo/dbus/exported_property_set_unittest.cc', + 'brillo/http/http_proxy_unittest.cc', + 'brillo/type_name_undecorate_unittest.cc', + 'brillo/variant_dictionary_unittest.cc', + '<(proto_in_dir)/test.proto', + ], + }], + ], + }, + { + 'target_name': 'libinstallattributes-<(libbase_ver)_unittests', + 'type': 'executable', + 'dependencies': [ + '../common-mk/external_dependencies.gyp:install_attributes-proto', + 'libinstallattributes-<(libbase_ver)', + ], + 'includes': ['../common-mk/common_test.gypi'], + 'sources': [ + 'install_attributes/tests/libinstallattributes_unittest.cc', ] }, { 'target_name': 'libpolicy-<(libbase_ver)_unittests', 'type': 'executable', - 'dependencies': ['libpolicy-<(libbase_ver)'], + 'dependencies': [ + '../common-mk/external_dependencies.gyp:install_attributes-proto', + '../common-mk/external_dependencies.gyp:policy-protos', + 'libinstallattributes-<(libbase_ver)', + 'libpolicy-<(libbase_ver)', + ], 'includes': ['../common-mk/common_test.gypi'], 'sources': [ + 'install_attributes/mock_install_attributes_reader.cc', + 'policy/tests/device_policy_impl_unittest.cc', 'policy/tests/libpolicy_unittest.cc', 'policy/tests/policy_util_unittest.cc', 'policy/tests/resilient_policy_util_unittest.cc', diff --git a/libinstallattributes.gypi b/libinstallattributes.gypi new file mode 100644 index 0000000..e0c0014 --- /dev/null +++ b/libinstallattributes.gypi @@ -0,0 +1,16 @@ +{ + 'targets': [ + { + 'target_name': 'libinstallattributes-includes', + 'type': 'none', + 'copies': [ + { + 'destination': '<(SHARED_INTERMEDIATE_DIR)/include/install_attributes', + 'files': [ + 'install_attributes/libinstallattributes.h', + ], + }, + ], + }, + ], +} diff --git a/policy/device_policy.h b/policy/device_policy.h index 2a3cb4b..5913d8c 100644 --- a/policy/device_policy.h +++ b/policy/device_policy.h @@ -9,9 +9,11 @@ #include <set> #include <string> +#include <utility> #include <vector> #include <base/macros.h> +#include <base/time/time.h> #pragma GCC visibility push(default) @@ -35,10 +37,35 @@ class DevicePolicy { uint16_t product_id; }; + // Time interval represented by two |day_of_week| and |time| pairs. The start + // of the interval is inclusive and the end is exclusive. The time represented + // by those pairs will be interpreted to be in the local timezone. Because of + // this, there exists the possibility of intervals being repeated or skipped + // in a day with daylight savings transitions, this is expected behavior. + struct WeeklyTimeInterval { + // Value is from 1 to 7 (1 = Monday, 2 = Tuesday, etc.). All values outside + // this range are invalid and will be discarded. + int start_day_of_week; + // Time since the start of the day. This value will be interpreted to be in + // the system's current timezone when used for range checking. + base::TimeDelta start_time; + int end_day_of_week; + base::TimeDelta end_time; + }; + + // Identifies a <day, percentage> pair in a staging schedule. + struct DayPercentagePair { + bool operator==(const DayPercentagePair& other) const { + return days == other.days && percentage == other.percentage; + } + int days; + int percentage; + }; + DevicePolicy(); virtual ~DevicePolicy(); - // Load the signed policy off of disk into |policy_|. + // Load device policy off of disk into |policy_|. // Returns true unless there is a policy on disk and loading it fails. virtual bool LoadPolicy() = 0; @@ -90,7 +117,7 @@ class DevicePolicy { // Writes the value of the EphemeralUsersEnabled policy in // |ephemeral_users_enabled|. Returns true on success. virtual bool GetEphemeralUsersEnabled( - bool* ephemeral_users_enabled) const = 0; + bool* ephemeral_users_enabled) const = 0; // Writes the value of the release channel policy in |release_channel|. // Returns true on success. @@ -110,6 +137,18 @@ class DevicePolicy { virtual bool GetTargetVersionPrefix( std::string* target_version_prefix) const = 0; + // Writes the value of the rollback_to_target_version policy in + // |rollback_to_target_version|. |rollback_to_target_version| will be one of + // the values in AutoUpdateSettingsProto's RollbackToTargetVersion enum. + // Returns true on success. + virtual bool GetRollbackToTargetVersion( + int* rollback_to_target_version) const = 0; + + // Writes the value of the rollback_allowed_milestones policy in + // |rollback_allowed_milestones|. Returns true on success. + virtual bool GetRollbackAllowedMilestones( + int* rollback_allowed_milestones) const = 0; + // Writes the value of the scatter_factor_in_seconds policy in // |scatter_factor_in_seconds|. Returns true on success. virtual bool GetScatterFactorInSeconds( @@ -152,8 +191,38 @@ class DevicePolicy { // Writes the value of the kiosk app id into |app_id_out|. // Only succeeds if the device is in auto-launched kiosk mode. - virtual bool GetAutoLaunchedKioskAppId( - std::string* app_id_out) const = 0; + virtual bool GetAutoLaunchedKioskAppId(std::string* app_id_out) const = 0; + + // Returns true if the policy data indicates that the device is enterprise + // managed. Note that this potentially could be faked by an exploit, therefore + // InstallAttributesReader must be used when tamper-proof evidence of the + // management state is required. + virtual bool IsEnterpriseManaged() const = 0; + + // Writes the value of the DeviceSecondFactorAuthentication policy in + // |mode_out|. |mode_out| is one of the values from + // DeviceSecondFactorAuthenticationProto's U2fMode enum (e.g. DISABLED, + // U2F or U2F_EXTENDED). Returns true on success. + virtual bool GetSecondFactorAuthenticationMode(int* mode_out) const = 0; + + // Writes the valid time intervals to |intervals_out|. These + // intervals are taken from the disallowed time intervals field in the + // AutoUpdateSettingsProto. Returns true if the intervals in the proto are + // valid. + virtual bool GetDisallowedTimeIntervals( + std::vector<WeeklyTimeInterval>* intervals_out) const = 0; + + // Writes the value of the DeviceUpdateStagingSchedule policy to + // |staging_schedule_out|. Returns true on success. + // The schedule is a list of <days, percentage> pairs. The percentages are + // expected to be mononically increasing in the range of [1, 100]. Similarly, + // days are expected to be monotonically increasing in the range [1, 28]. Each + // pair describes the |percentage| of the fleet that is expected to receive an + // update after |days| days after an update was discovered. e.g. [<4, 30>, <8, + // 100>] means that 30% of devices should be updated in the first 4 days, and + // then 100% should be updated after 8 days. + virtual bool GetDeviceUpdateStagingSchedule( + std::vector<DayPercentagePair>* staging_schedule_out) const = 0; private: // Verifies that the policy signature is correct. diff --git a/policy/device_policy_impl.cc b/policy/device_policy_impl.cc index ec52be6..76b82a1 100644 --- a/policy/device_policy_impl.cc +++ b/policy/device_policy_impl.cc @@ -4,26 +4,34 @@ #include "policy/device_policy_impl.h" +#include <algorithm> #include <memory> +#include <set> +#include <string> #include <base/containers/adapters.h> #include <base/files/file_util.h> +#include <base/json/json_reader.h> #include <base/logging.h> #include <base/macros.h> #include <base/memory/ptr_util.h> +#include <base/time/time.h> +#include <base/values.h> #include <openssl/evp.h> #include <openssl/x509.h> -#include <set> -#include <string> - #include "bindings/chrome_device_policy.pb.h" #include "bindings/device_management_backend.pb.h" #include "policy/policy_util.h" #include "policy/resilient_policy_util.h" +namespace em = enterprise_management; + namespace policy { +// Maximum value of RollbackAllowedMilestones policy. +const int kMaxRollbackAllowedMilestones = 4; + namespace { const char kPolicyPath[] = "/var/lib/whitelist/policy"; const char kPublicKeyPath[] = "/var/lib/whitelist/owner.key"; @@ -93,12 +101,76 @@ std::string DecodeConnectionType(int type) { return kConnectionTypes[type]; } +// TODO(adokar): change type to base::Optional<int> when available. +int ConvertDayOfWeekStringToInt(const std::string& day_of_week_str) { + if (day_of_week_str == "Sunday") return 0; + if (day_of_week_str == "Monday") return 1; + if (day_of_week_str == "Tuesday") return 2; + if (day_of_week_str == "Wednesday") return 3; + if (day_of_week_str == "Thursday") return 4; + if (day_of_week_str == "Friday") return 5; + if (day_of_week_str == "Saturday") return 6; + return -1; +} + +bool DecodeWeeklyTimeFromValue(const base::DictionaryValue& dict_value, + int* day_of_week_out, + base::TimeDelta* time_out) { + std::string day_of_week_str; + if (!dict_value.GetString("day_of_week", &day_of_week_str)) { + LOG(ERROR) << "Day of the week is absent."; + return false; + } + *day_of_week_out = ConvertDayOfWeekStringToInt(day_of_week_str); + if (*day_of_week_out == -1) { + LOG(ERROR) << "Undefined day of the week: " << day_of_week_str; + return false; + } + + int hours; + if (!dict_value.GetInteger("hours", &hours) || hours < 0 || hours > 23) { + LOG(ERROR) << "Hours are absent or are outside of the range [0, 24)."; + return false; + } + + int minutes; + if (!dict_value.GetInteger("minutes", &minutes) || minutes < 0 || + minutes > 59) { + LOG(ERROR) << "Minutes are absent or are outside the range [0, 60)"; + return false; + } + + *time_out = + base::TimeDelta::FromMinutes(minutes) + base::TimeDelta::FromHours(hours); + return true; +} + +std::unique_ptr<base::ListValue> DecodeListValueFromJSON( + const std::string& json_string) { + std::string error; + std::unique_ptr<base::Value> decoded_json = + base::JSONReader::ReadAndReturnError(json_string, + base::JSON_ALLOW_TRAILING_COMMAS, + nullptr, &error); + if (!decoded_json) { + LOG(ERROR) << "Invalid JSON string: " << error; + return nullptr; + } + + std::unique_ptr<base::ListValue> list_val = + base::ListValue::From(std::move(decoded_json)); + if (!list_val) { + LOG(ERROR) << "JSON string is not a list"; + return nullptr; + } + + return list_val; +} + } // namespace DevicePolicyImpl::DevicePolicyImpl() - : policy_path_(kPolicyPath), - keyfile_path_(kPublicKeyPath), - verify_root_ownership_(true) {} + : policy_path_(kPolicyPath), keyfile_path_(kPublicKeyPath) {} DevicePolicyImpl::~DevicePolicyImpl() {} @@ -136,8 +208,7 @@ bool DevicePolicyImpl::GetUserWhitelist( std::vector<std::string>* user_whitelist) const { if (!device_policy_.has_user_whitelist()) return false; - const enterprise_management::UserWhitelistProto& proto = - device_policy_.user_whitelist(); + const em::UserWhitelistProto& proto = device_policy_.user_whitelist(); user_whitelist->clear(); for (int i = 0; i < proto.user_whitelist_size(); i++) user_whitelist->push_back(proto.user_whitelist(i)); @@ -192,8 +263,7 @@ bool DevicePolicyImpl::GetReportVersionInfo(bool* report_version_info) const { if (!device_policy_.has_device_reporting()) return false; - const enterprise_management::DeviceReportingProto& proto = - device_policy_.device_reporting(); + const em::DeviceReportingProto& proto = device_policy_.device_reporting(); if (!proto.has_report_version_info()) return false; @@ -206,8 +276,7 @@ bool DevicePolicyImpl::GetReportActivityTimes( if (!device_policy_.has_device_reporting()) return false; - const enterprise_management::DeviceReportingProto& proto = - device_policy_.device_reporting(); + const em::DeviceReportingProto& proto = device_policy_.device_reporting(); if (!proto.has_report_activity_times()) return false; @@ -219,8 +288,7 @@ bool DevicePolicyImpl::GetReportBootMode(bool* report_boot_mode) const { if (!device_policy_.has_device_reporting()) return false; - const enterprise_management::DeviceReportingProto& proto = - device_policy_.device_reporting(); + const em::DeviceReportingProto& proto = device_policy_.device_reporting(); if (!proto.has_report_boot_mode()) return false; @@ -241,8 +309,7 @@ bool DevicePolicyImpl::GetReleaseChannel(std::string* release_channel) const { if (!device_policy_.has_release_channel()) return false; - const enterprise_management::ReleaseChannelProto& proto = - device_policy_.release_channel(); + const em::ReleaseChannelProto& proto = device_policy_.release_channel(); if (!proto.has_release_channel()) return false; @@ -255,8 +322,7 @@ bool DevicePolicyImpl::GetReleaseChannelDelegated( if (!device_policy_.has_release_channel()) return false; - const enterprise_management::ReleaseChannelProto& proto = - device_policy_.release_channel(); + const em::ReleaseChannelProto& proto = device_policy_.release_channel(); if (!proto.has_release_channel_delegated()) return false; @@ -268,7 +334,7 @@ bool DevicePolicyImpl::GetUpdateDisabled(bool* update_disabled) const { if (!device_policy_.has_auto_update_settings()) return false; - const enterprise_management::AutoUpdateSettingsProto& proto = + const em::AutoUpdateSettingsProto& proto = device_policy_.auto_update_settings(); if (!proto.has_update_disabled()) return false; @@ -282,7 +348,7 @@ bool DevicePolicyImpl::GetTargetVersionPrefix( if (!device_policy_.has_auto_update_settings()) return false; - const enterprise_management::AutoUpdateSettingsProto& proto = + const em::AutoUpdateSettingsProto& proto = device_policy_.auto_update_settings(); if (!proto.has_target_version_prefix()) return false; @@ -291,12 +357,58 @@ bool DevicePolicyImpl::GetTargetVersionPrefix( return true; } +bool DevicePolicyImpl::GetRollbackToTargetVersion( + int* rollback_to_target_version) const { + if (!device_policy_.has_auto_update_settings()) + return false; + + const em::AutoUpdateSettingsProto& proto = + device_policy_.auto_update_settings(); + if (!proto.has_rollback_to_target_version()) + return false; + + *rollback_to_target_version = proto.rollback_to_target_version(); + return true; +} + +bool DevicePolicyImpl::GetRollbackAllowedMilestones( + int* rollback_allowed_milestones) const { + // This policy can be only set for devices which are enterprise enrolled. + if (!install_attributes_reader_->IsLocked()) + return false; + if (install_attributes_reader_->GetAttribute( + InstallAttributesReader::kAttrMode) != + InstallAttributesReader::kDeviceModeEnterprise && + install_attributes_reader_->GetAttribute( + InstallAttributesReader::kAttrMode) != + InstallAttributesReader::kDeviceModeEnterpriseAD) + return false; + + if (device_policy_.has_auto_update_settings()) { + const em::AutoUpdateSettingsProto& proto = + device_policy_.auto_update_settings(); + if (proto.has_rollback_allowed_milestones()) { + // Policy is set, enforce minimum and maximum constraints. + *rollback_allowed_milestones = proto.rollback_allowed_milestones(); + if (*rollback_allowed_milestones < 0) + *rollback_allowed_milestones = 0; + if (*rollback_allowed_milestones > kMaxRollbackAllowedMilestones) + *rollback_allowed_milestones = kMaxRollbackAllowedMilestones; + return true; + } + } + // Policy is not present, use default for enterprise devices. + VLOG(1) << "RollbackAllowedMilestones policy is not set, using default 0."; + *rollback_allowed_milestones = 0; + return true; +} + bool DevicePolicyImpl::GetScatterFactorInSeconds( int64_t* scatter_factor_in_seconds) const { if (!device_policy_.has_auto_update_settings()) return false; - const enterprise_management::AutoUpdateSettingsProto& proto = + const em::AutoUpdateSettingsProto& proto = device_policy_.auto_update_settings(); if (!proto.has_scatter_factor_in_seconds()) return false; @@ -310,7 +422,7 @@ bool DevicePolicyImpl::GetAllowedConnectionTypesForUpdate( if (!device_policy_.has_auto_update_settings()) return false; - const enterprise_management::AutoUpdateSettingsProto& proto = + const em::AutoUpdateSettingsProto& proto = device_policy_.auto_update_settings(); if (proto.allowed_connection_types_size() <= 0) return false; @@ -328,7 +440,7 @@ bool DevicePolicyImpl::GetOpenNetworkConfiguration( if (!device_policy_.has_open_network_configuration()) return false; - const enterprise_management::DeviceOpenNetworkConfigurationProto& proto = + const em::DeviceOpenNetworkConfigurationProto& proto = device_policy_.open_network_configuration(); if (!proto.has_open_network_configuration()) return false; @@ -338,11 +450,11 @@ bool DevicePolicyImpl::GetOpenNetworkConfiguration( } bool DevicePolicyImpl::GetOwner(std::string* owner) const { - // The device is enterprise enrolled iff a request token exists. - if (policy_data_.has_request_token()) { + if (IsEnterpriseManaged()) { *owner = ""; return true; } + if (!policy_data_.has_username()) return false; *owner = policy_data_.username(); @@ -354,7 +466,7 @@ bool DevicePolicyImpl::GetHttpDownloadsEnabled( if (!device_policy_.has_auto_update_settings()) return false; - const enterprise_management::AutoUpdateSettingsProto& proto = + const em::AutoUpdateSettingsProto& proto = device_policy_.auto_update_settings(); if (!proto.has_http_downloads_enabled()) @@ -368,7 +480,7 @@ bool DevicePolicyImpl::GetAuP2PEnabled(bool* au_p2p_enabled) const { if (!device_policy_.has_auto_update_settings()) return false; - const enterprise_management::AutoUpdateSettingsProto& proto = + const em::AutoUpdateSettingsProto& proto = device_policy_.auto_update_settings(); if (!proto.has_p2p_enabled()) @@ -383,7 +495,7 @@ bool DevicePolicyImpl::GetAllowKioskAppControlChromeVersion( if (!device_policy_.has_allow_kiosk_app_control_chrome_version()) return false; - const enterprise_management::AllowKioskAppControlChromeVersionProto& proto = + const em::AllowKioskAppControlChromeVersionProto& proto = device_policy_.allow_kiosk_app_control_chrome_version(); if (!proto.has_allow_kiosk_app_control_chrome_version()) @@ -398,11 +510,11 @@ bool DevicePolicyImpl::GetUsbDetachableWhitelist( std::vector<UsbDeviceId>* usb_whitelist) const { if (!device_policy_.has_usb_detachable_whitelist()) return false; - const enterprise_management::UsbDetachableWhitelistProto& proto = + const em::UsbDetachableWhitelistProto& proto = device_policy_.usb_detachable_whitelist(); usb_whitelist->clear(); for (int i = 0; i < proto.id_size(); i++) { - const ::enterprise_management::UsbDeviceIdProto& id = proto.id(i); + const em::UsbDeviceIdProto& id = proto.id(i); UsbDeviceId dev_id; dev_id.vendor_id = id.has_vendor_id() ? id.vendor_id() : 0; dev_id.product_id = id.has_product_id() ? id.product_id() : 0; @@ -411,12 +523,46 @@ bool DevicePolicyImpl::GetUsbDetachableWhitelist( return true; } +bool DevicePolicyImpl::GetDeviceUpdateStagingSchedule( + std::vector<DayPercentagePair>* staging_schedule_out) const { + staging_schedule_out->clear(); + + if (!device_policy_.has_auto_update_settings()) + return false; + + const em::AutoUpdateSettingsProto &proto = + device_policy_.auto_update_settings(); + + if (!proto.has_staging_schedule()) + return false; + + std::unique_ptr<base::ListValue> list_val = + DecodeListValueFromJSON(proto.staging_schedule()); + if (!list_val) + return false; + + for (base::Value* const& pair_value : *list_val) { + base::DictionaryValue* day_percentage_pair; + if (!pair_value->GetAsDictionary(&day_percentage_pair)) + return false; + int days, percentage; + if (!day_percentage_pair->GetInteger("days", &days) || + !day_percentage_pair->GetInteger("percentage", &percentage)) + return false; + // Limit the percentage to [0, 100] and days to [1, 28]; + staging_schedule_out->push_back({std::max(std::min(days, 28), 1), + std::max(std::min(percentage, 100), 0)}); + } + + return true; +} + bool DevicePolicyImpl::GetAutoLaunchedKioskAppId( std::string* app_id_out) const { if (!device_policy_.has_device_local_accounts()) return false; - const enterprise_management::DeviceLocalAccountsProto& local_accounts = + const em::DeviceLocalAccountsProto& local_accounts = device_policy_.device_local_accounts(); // For auto-launched kiosk apps, the delay needs to be 0. @@ -424,7 +570,7 @@ bool DevicePolicyImpl::GetAutoLaunchedKioskAppId( local_accounts.auto_login_delay() != 0) return false; - for (const enterprise_management::DeviceLocalAccountInfoProto& account : + for (const em::DeviceLocalAccountInfoProto& account : local_accounts.account()) { // If this isn't an auto-login account, move to the next one. if (account.account_id() != local_accounts.auto_login_id()) @@ -433,8 +579,7 @@ bool DevicePolicyImpl::GetAutoLaunchedKioskAppId( // If the auto-launched account is not a kiosk app, bail out, we aren't // running in auto-launched kiosk mode. if (account.type() != - enterprise_management::DeviceLocalAccountInfoProto:: - ACCOUNT_TYPE_KIOSK_APP) { + em::DeviceLocalAccountInfoProto::ACCOUNT_TYPE_KIOSK_APP) { return false; } @@ -446,6 +591,74 @@ bool DevicePolicyImpl::GetAutoLaunchedKioskAppId( return false; } +bool DevicePolicyImpl::IsEnterpriseManaged() const { + if (policy_data_.has_management_mode()) + return policy_data_.management_mode() == em::PolicyData::ENTERPRISE_MANAGED; + // Fall back to checking the request token, see management_mode documentation + // in device_management_backend.proto. + return policy_data_.has_request_token(); +} + +bool DevicePolicyImpl::GetSecondFactorAuthenticationMode(int* mode_out) const { + if (!device_policy_.has_device_second_factor_authentication()) + return false; + + const em::DeviceSecondFactorAuthenticationProto& proto = + device_policy_.device_second_factor_authentication(); + + if (!proto.has_mode()) + return false; + + *mode_out = proto.mode(); + return true; +} + +bool DevicePolicyImpl::GetDisallowedTimeIntervals( + std::vector<WeeklyTimeInterval>* intervals_out) const { + intervals_out->clear(); + + if (!device_policy_.has_auto_update_settings()) { + return false; + } + + const em::AutoUpdateSettingsProto& proto = + device_policy_.auto_update_settings(); + + if (!proto.has_disallowed_time_intervals()) { + return false; + } + + std::unique_ptr<base::ListValue> list_val = + DecodeListValueFromJSON(proto.disallowed_time_intervals()); + if (!list_val) + return false; + + for (base::Value* const& interval_value : *list_val) { + base::DictionaryValue* interval_dict; + if (!interval_value->GetAsDictionary(&interval_dict)) { + LOG(ERROR) << "Invalid JSON string given. Interval is not a dict."; + return false; + } + base::DictionaryValue* start; + base::DictionaryValue* end; + if (!interval_dict->GetDictionary("start", &start) || + !interval_dict->GetDictionary("end", &end)) { + LOG(ERROR) << "Interval is missing start/end."; + return false; + } + WeeklyTimeInterval weekly_interval; + if (!DecodeWeeklyTimeFromValue(*start, &weekly_interval.start_day_of_week, + &weekly_interval.start_time) || + !DecodeWeeklyTimeFromValue(*end, &weekly_interval.end_day_of_week, + &weekly_interval.end_time)) { + return false; + } + + intervals_out->push_back(weekly_interval); + } + return true; +} + bool DevicePolicyImpl::VerifyPolicyFile(const base::FilePath& policy_path) { if (!verify_root_ownership_) { return true; @@ -506,7 +719,7 @@ bool DevicePolicyImpl::LoadPolicyFromFile(const base::FilePath& policy_path) { return false; } - bool verify_policy = true; + bool verify_policy = verify_policy_; if (!install_attributes_reader_) { install_attributes_reader_ = std::make_unique<InstallAttributesReader>(); } diff --git a/policy/device_policy_impl.h b/policy/device_policy_impl.h index b178e28..6891312 100644 --- a/policy/device_policy_impl.h +++ b/policy/device_policy_impl.h @@ -5,8 +5,10 @@ #ifndef LIBBRILLO_POLICY_DEVICE_POLICY_IMPL_H_ #define LIBBRILLO_POLICY_DEVICE_POLICY_IMPL_H_ +#include <memory> #include <set> #include <string> +#include <utility> #include <vector> #include <base/files/file_path.h> @@ -14,6 +16,7 @@ #include "bindings/chrome_device_policy.pb.h" #include "bindings/device_management_backend.pb.h" +#include "install_attributes/libinstallattributes.h" #include "policy/device_policy.h" #pragma GCC visibility push(default) @@ -30,6 +33,12 @@ class DevicePolicyImpl : public DevicePolicy { DevicePolicyImpl(); ~DevicePolicyImpl() override; + const enterprise_management::ChromeDeviceSettingsProto& get_device_policy() + const { + return device_policy_; + } + + // DevicePolicy overrides: bool LoadPolicy() override; bool GetPolicyRefreshRate(int* rate) const override; bool GetUserWhitelist( @@ -50,6 +59,10 @@ class DevicePolicyImpl : public DevicePolicy { bool GetUpdateDisabled(bool* update_disabled) const override; bool GetTargetVersionPrefix( std::string* target_version_prefix) const override; + bool GetRollbackToTargetVersion( + int* rollback_to_target_version) const override; + bool GetRollbackAllowedMilestones( + int* rollback_allowed_milestones) const override; bool GetScatterFactorInSeconds( int64_t* scatter_factor_in_seconds) const override; bool GetAllowedConnectionTypesForUpdate( @@ -64,6 +77,12 @@ class DevicePolicyImpl : public DevicePolicy { bool GetUsbDetachableWhitelist( std::vector<UsbDeviceId>* usb_whitelist) const override; bool GetAutoLaunchedKioskAppId(std::string* app_id_out) const override; + bool IsEnterpriseManaged() const override; + bool GetSecondFactorAuthenticationMode(int* mode_out) const override; + bool GetDisallowedTimeIntervals( + std::vector<WeeklyTimeInterval>* intervals_out) const override; + bool GetDeviceUpdateStagingSchedule( + std::vector<DayPercentagePair> *staging_schedule_out) const override; // Methods that can be used only for testing. void set_policy_data_for_testing( @@ -77,12 +96,17 @@ class DevicePolicyImpl : public DevicePolicy { std::unique_ptr<InstallAttributesReader> install_attributes_reader) { install_attributes_reader_ = std::move(install_attributes_reader); } + void set_policy_for_testing( + const enterprise_management::ChromeDeviceSettingsProto& device_policy) { + device_policy_ = device_policy; + } void set_policy_path_for_testing(const base::FilePath& policy_path) { policy_path_ = policy_path; } void set_key_file_path_for_testing(const base::FilePath& keyfile_path) { keyfile_path_ = keyfile_path; } + void set_verify_policy_for_testing(bool value) { verify_policy_ = value; } private: // Verifies that both the policy file and the signature file exist and are @@ -93,11 +117,14 @@ class DevicePolicyImpl : public DevicePolicy { // Verifies that the policy signature is correct. bool VerifyPolicySignature() override; - // Loads the signed policy off of disk from |policy_path| into |policy_|. - // Returns true if the |policy_path| is present on disk and loading it is - // successful. + // Loads policy off of disk from |policy_path| into |policy_|. Returns true if + // the |policy_path| is present on disk and loading it is successful. bool LoadPolicyFromFile(const base::FilePath& policy_path); + // Path of the default policy file, e.g. /path/to/policy. In order to make + // device policy more resilient against broken files, this class also tries to + // load indexed paths /path/to/policy.1, /path/to/policy.2 etc., see + // resilient_policy_utils.h. base::FilePath policy_path_; base::FilePath keyfile_path_; std::unique_ptr<InstallAttributesReader> install_attributes_reader_; @@ -107,7 +134,10 @@ class DevicePolicyImpl : public DevicePolicy { // If true, verify that policy files are owned by root. True in production // but can be set to false by tests. - bool verify_root_ownership_; + bool verify_root_ownership_ = true; + // If false, all types of verification are disabled. True in production + // but can be set to false by tests. + bool verify_policy_ = true; DISALLOW_COPY_AND_ASSIGN(DevicePolicyImpl); }; diff --git a/policy/libpolicy.cc b/policy/libpolicy.cc index 99fa46d..a0b7640 100644 --- a/policy/libpolicy.cc +++ b/policy/libpolicy.cc @@ -4,6 +4,8 @@ #include "policy/libpolicy.h" +#include <memory> + #include <base/logging.h> #include "policy/device_policy.h" @@ -13,17 +15,23 @@ namespace policy { -PolicyProvider::PolicyProvider() - : device_policy_(nullptr), - device_policy_is_loaded_(false) { +PolicyProvider::PolicyProvider() { #ifndef __ANDROID__ - device_policy_.reset(new DevicePolicyImpl()); + device_policy_ = std::make_unique<DevicePolicyImpl>(); + install_attributes_reader_ = std::make_unique<InstallAttributesReader>(); #endif } PolicyProvider::PolicyProvider(std::unique_ptr<DevicePolicy> device_policy) : device_policy_(std::move(device_policy)), - device_policy_is_loaded_(true) {} +#ifdef __ANDROID__ + device_policy_is_loaded_(true) { +} +#else + device_policy_is_loaded_(true), + install_attributes_reader_(std::make_unique<InstallAttributesReader>()) { +} +#endif // __ANDROID__ PolicyProvider::~PolicyProvider() {} @@ -42,10 +50,34 @@ bool PolicyProvider::device_policy_is_loaded() const { } const DevicePolicy& PolicyProvider::GetDevicePolicy() const { - if (!device_policy_is_loaded_) - DCHECK("Trying to get policy data but policy was not loaded!"); - + DCHECK(device_policy_is_loaded_) + << "Trying to get policy data but policy was not loaded!"; return *device_policy_; } +bool PolicyProvider::IsConsumerDevice() const { +#ifdef __ANDROID__ + return true; +#else + if (!install_attributes_reader_->IsLocked()) + return false; + + const std::string& device_mode = install_attributes_reader_->GetAttribute( + InstallAttributesReader::kAttrMode); + return device_mode != InstallAttributesReader::kDeviceModeEnterprise && + device_mode != InstallAttributesReader::kDeviceModeEnterpriseAD; +#endif // __ANDROID__ +} + +void PolicyProvider::SetDevicePolicyForTesting( + std::unique_ptr<DevicePolicy> device_policy) { + device_policy_ = std::move(device_policy); + device_policy_is_loaded_ = true; +} + +void PolicyProvider::SetInstallAttributesReaderForTesting( + std::unique_ptr<InstallAttributesReader> install_attributes_reader) { + install_attributes_reader_ = std::move(install_attributes_reader); +} + } // namespace policy diff --git a/policy/libpolicy.h b/policy/libpolicy.h index 1a411cf..5e87f58 100644 --- a/policy/libpolicy.h +++ b/policy/libpolicy.h @@ -10,6 +10,8 @@ #include <base/macros.h> +#include "install_attributes/libinstallattributes.h" + #pragma GCC visibility push(default) namespace policy { @@ -22,6 +24,7 @@ class DevicePolicy; // its signature. class PolicyProvider { public: + // The default constructor does not load policy. PolicyProvider(); virtual ~PolicyProvider(); @@ -37,9 +40,20 @@ class PolicyProvider { // Returns a value from the device policy cache. virtual const DevicePolicy& GetDevicePolicy() const; + // Returns true if the device is not an enterprise enrolled device, so it + // won't have device policy before the next powerwash. Returns false if device + // is still in OOBE (so device mode is not determined yet). + virtual bool IsConsumerDevice() const; + + void SetDevicePolicyForTesting( + std::unique_ptr<DevicePolicy> device_policy); + void SetInstallAttributesReaderForTesting( + std::unique_ptr<InstallAttributesReader> install_attributes_reader); + private: std::unique_ptr<DevicePolicy> device_policy_; - bool device_policy_is_loaded_; + bool device_policy_is_loaded_ = false; + std::unique_ptr<InstallAttributesReader> install_attributes_reader_; DISALLOW_COPY_AND_ASSIGN(PolicyProvider); }; diff --git a/policy/mock_device_policy.h b/policy/mock_device_policy.h index 23e8147..90470e2 100644 --- a/policy/mock_device_policy.h +++ b/policy/mock_device_policy.h @@ -7,6 +7,7 @@ #include <set> #include <string> +#include <utility> #include <vector> #include <gmock/gmock.h> @@ -82,6 +83,8 @@ class MockDevicePolicy : public DevicePolicy { MOCK_CONST_METHOD1(GetUpdateDisabled, bool(bool*)); // NOLINT(readability/function) MOCK_CONST_METHOD1(GetTargetVersionPrefix, bool(std::string*)); + MOCK_CONST_METHOD1(GetRollbackToTargetVersion, bool(int*)); + MOCK_CONST_METHOD1(GetRollbackAllowedMilestones, bool(int*)); MOCK_CONST_METHOD1(GetScatterFactorInSeconds, bool(int64_t*)); // NOLINT(readability/function) MOCK_CONST_METHOD1(GetAllowedConnectionTypesForUpdate, @@ -97,7 +100,12 @@ class MockDevicePolicy : public DevicePolicy { MOCK_CONST_METHOD1(GetUsbDetachableWhitelist, bool(std::vector<DevicePolicy::UsbDeviceId>*)); MOCK_CONST_METHOD1(GetAutoLaunchedKioskAppId, bool(std::string*)); - + MOCK_CONST_METHOD0(IsEnterpriseManaged, bool()); + MOCK_CONST_METHOD1(GetSecondFactorAuthenticationMode, bool(int*)); + MOCK_CONST_METHOD1(GetDisallowedTimeIntervals, + bool(std::vector<WeeklyTimeInterval>*)); + MOCK_CONST_METHOD1(GetDeviceUpdateStagingSchedule, + bool(std::vector<DayPercentagePair>*)); MOCK_METHOD0(VerifyPolicyFiles, bool(void)); MOCK_METHOD0(VerifyPolicySignature, bool(void)); }; diff --git a/policy/mock_libpolicy.h b/policy/mock_libpolicy.h index acb9b8b..a0f6920 100644 --- a/policy/mock_libpolicy.h +++ b/policy/mock_libpolicy.h @@ -23,6 +23,7 @@ class MockPolicyProvider : public PolicyProvider { MOCK_METHOD0(Reload, bool(void)); MOCK_CONST_METHOD0(device_policy_is_loaded, bool(void)); MOCK_CONST_METHOD0(GetDevicePolicy, const DevicePolicy&(void)); + MOCK_CONST_METHOD0(IsConsumerDevice, bool(void)); private: DISALLOW_COPY_AND_ASSIGN(MockPolicyProvider); diff --git a/policy/tests/device_policy_impl_unittest.cc b/policy/tests/device_policy_impl_unittest.cc new file mode 100644 index 0000000..37c3916 --- /dev/null +++ b/policy/tests/device_policy_impl_unittest.cc @@ -0,0 +1,250 @@ +// Copyright 2017 The Chromium OS 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 <gmock/gmock.h> +#include <gtest/gtest.h> + +#include "policy/device_policy_impl.h" + +#include "bindings/chrome_device_policy.pb.h" +#include "install_attributes/mock_install_attributes_reader.h" + +namespace em = enterprise_management; + +using testing::ElementsAre; + +namespace policy { + +class DevicePolicyImplTest : public testing::Test, public DevicePolicyImpl { + protected: + void InitializePolicy(const char* device_mode, + const em::ChromeDeviceSettingsProto& proto) { + device_policy_.set_policy_for_testing(proto); + device_policy_.set_install_attributes_for_testing( + std::make_unique<MockInstallAttributesReader>( + device_mode, true /* initialized */)); + } + + DevicePolicyImpl device_policy_; +}; + +// Enterprise managed. +TEST_F(DevicePolicyImplTest, GetOwner_Managed) { + em::PolicyData policy_data; + policy_data.set_username("user@example.com"); + policy_data.set_management_mode(em::PolicyData::ENTERPRISE_MANAGED); + device_policy_.set_policy_data_for_testing(policy_data); + + std::string owner("something"); + EXPECT_TRUE(device_policy_.GetOwner(&owner)); + EXPECT_TRUE(owner.empty()); +} + +// Consumer owned. +TEST_F(DevicePolicyImplTest, GetOwner_Consumer) { + em::PolicyData policy_data; + policy_data.set_username("user@example.com"); + policy_data.set_management_mode(em::PolicyData::LOCAL_OWNER); + policy_data.set_request_token("codepath-must-ignore-dmtoken"); + device_policy_.set_policy_data_for_testing(policy_data); + + std::string owner; + EXPECT_TRUE(device_policy_.GetOwner(&owner)); + EXPECT_EQ("user@example.com", owner); +} + +// Consumer owned, username is missing. +TEST_F(DevicePolicyImplTest, GetOwner_ConsumerMissingUsername) { + em::PolicyData policy_data; + device_policy_.set_policy_data_for_testing(policy_data); + + std::string owner("something"); + EXPECT_FALSE(device_policy_.GetOwner(&owner)); + EXPECT_EQ("something", owner); +} + +// Enterprise managed, denoted by management_mode. +TEST_F(DevicePolicyImplTest, IsEnterpriseManaged_ManagementModeManaged) { + em::PolicyData policy_data; + policy_data.set_management_mode(em::PolicyData::ENTERPRISE_MANAGED); + device_policy_.set_policy_data_for_testing(policy_data); + + EXPECT_TRUE(device_policy_.IsEnterpriseManaged()); +} + +// Enterprise managed, fallback to DM token. +TEST_F(DevicePolicyImplTest, IsEnterpriseManaged_DMTokenManaged) { + em::PolicyData policy_data; + policy_data.set_request_token("abc"); + device_policy_.set_policy_data_for_testing(policy_data); + + EXPECT_TRUE(device_policy_.IsEnterpriseManaged()); +} + +// Consumer owned, denoted by management_mode. +TEST_F(DevicePolicyImplTest, IsEnterpriseManaged_ManagementModeConsumer) { + em::PolicyData policy_data; + policy_data.set_management_mode(em::PolicyData::LOCAL_OWNER); + policy_data.set_request_token("codepath-must-ignore-dmtoken"); + device_policy_.set_policy_data_for_testing(policy_data); + + EXPECT_FALSE(device_policy_.IsEnterpriseManaged()); +} + +// Consumer owned, fallback to interpreting absence of DM token. +TEST_F(DevicePolicyImplTest, IsEnterpriseManaged_DMTokenConsumer) { + em::PolicyData policy_data; + device_policy_.set_policy_data_for_testing(policy_data); + + EXPECT_FALSE(device_policy_.IsEnterpriseManaged()); +} + +// RollbackAllowedMilestones is not set. +TEST_F(DevicePolicyImplTest, GetRollbackAllowedMilestones_NotSet) { + device_policy_.set_install_attributes_for_testing( + std::make_unique<MockInstallAttributesReader>( + InstallAttributesReader::kDeviceModeEnterprise, true)); + + int value = -1; + ASSERT_TRUE(device_policy_.GetRollbackAllowedMilestones(&value)); + EXPECT_EQ(0, value); +} + +// RollbackAllowedMilestones is set to a valid value. +TEST_F(DevicePolicyImplTest, GetRollbackAllowedMilestones_Set) { + em::ChromeDeviceSettingsProto device_policy_proto; + em::AutoUpdateSettingsProto* auto_update_settings = + device_policy_proto.mutable_auto_update_settings(); + auto_update_settings->set_rollback_allowed_milestones(3); + InitializePolicy(InstallAttributesReader::kDeviceModeEnterprise, + device_policy_proto); + + int value = -1; + ASSERT_TRUE(device_policy_.GetRollbackAllowedMilestones(&value)); + EXPECT_EQ(3, value); +} + +// RollbackAllowedMilestones is set to a valid value, using AD. +TEST_F(DevicePolicyImplTest, GetRollbackAllowedMilestones_SetAD) { + em::ChromeDeviceSettingsProto device_policy_proto; + em::AutoUpdateSettingsProto* auto_update_settings = + device_policy_proto.mutable_auto_update_settings(); + auto_update_settings->set_rollback_allowed_milestones(3); + InitializePolicy(InstallAttributesReader::kDeviceModeEnterpriseAD, + device_policy_proto); + int value = -1; + ASSERT_TRUE(device_policy_.GetRollbackAllowedMilestones(&value)); + EXPECT_EQ(3, value); +} + +// RollbackAllowedMilestones is set to a valid value, but it's not an enterprise +// device. +TEST_F(DevicePolicyImplTest, GetRollbackAllowedMilestones_SetConsumer) { + em::ChromeDeviceSettingsProto device_policy_proto; + em::AutoUpdateSettingsProto* auto_update_settings = + device_policy_proto.mutable_auto_update_settings(); + auto_update_settings->set_rollback_allowed_milestones(3); + InitializePolicy(InstallAttributesReader::kDeviceModeConsumer, + device_policy_proto); + + int value = -1; + ASSERT_FALSE(device_policy_.GetRollbackAllowedMilestones(&value)); +} + +// RollbackAllowedMilestones is set to an invalid value. +TEST_F(DevicePolicyImplTest, GetRollbackAllowedMilestones_SetTooLarge) { + em::ChromeDeviceSettingsProto device_policy_proto; + em::AutoUpdateSettingsProto* auto_update_settings = + device_policy_proto.mutable_auto_update_settings(); + auto_update_settings->set_rollback_allowed_milestones(10); + InitializePolicy(InstallAttributesReader::kDeviceModeEnterprise, + device_policy_proto); + + int value = -1; + ASSERT_TRUE(device_policy_.GetRollbackAllowedMilestones(&value)); + EXPECT_EQ(4, value); +} + +// RollbackAllowedMilestones is set to an invalid value. +TEST_F(DevicePolicyImplTest, GetRollbackAllowedMilestones_SetTooSmall) { + em::ChromeDeviceSettingsProto device_policy_proto; + em::AutoUpdateSettingsProto* auto_update_settings = + device_policy_proto.mutable_auto_update_settings(); + auto_update_settings->set_rollback_allowed_milestones(-1); + InitializePolicy(InstallAttributesReader::kDeviceModeEnterprise, + device_policy_proto); + + int value = -1; + ASSERT_TRUE(device_policy_.GetRollbackAllowedMilestones(&value)); + EXPECT_EQ(0, value); +} + +// Update staging schedule has no values +TEST_F(DevicePolicyImplTest, GetDeviceUpdateStagingSchedule_NoValues) { + em::ChromeDeviceSettingsProto device_policy_proto; + em::AutoUpdateSettingsProto *auto_update_settings = + device_policy_proto.mutable_auto_update_settings(); + auto_update_settings->set_staging_schedule("[]"); + InitializePolicy(InstallAttributesReader::kDeviceModeEnterprise, + device_policy_proto); + + std::vector<DayPercentagePair> staging_schedule; + ASSERT_TRUE(device_policy_.GetDeviceUpdateStagingSchedule(&staging_schedule)); + EXPECT_EQ(0, staging_schedule.size()); +} + +// Update staging schedule has valid values +TEST_F(DevicePolicyImplTest, GetDeviceUpdateStagingSchedule_Valid) { + em::ChromeDeviceSettingsProto device_policy_proto; + em::AutoUpdateSettingsProto *auto_update_settings = + device_policy_proto.mutable_auto_update_settings(); + auto_update_settings->set_staging_schedule( + "[{\"days\": 4, \"percentage\": 40}, {\"days\": 10, \"percentage\": " + "100}]"); + InitializePolicy(InstallAttributesReader::kDeviceModeEnterprise, + device_policy_proto); + + std::vector<DayPercentagePair> staging_schedule; + ASSERT_TRUE(device_policy_.GetDeviceUpdateStagingSchedule(&staging_schedule)); + EXPECT_THAT(staging_schedule, ElementsAre(DayPercentagePair{4, 40}, + DayPercentagePair{10, 100})); +} + +// Update staging schedule has valid values, set using AD. +TEST_F(DevicePolicyImplTest, GetDeviceUpdateStagingSchedule_Valid_AD) { + em::ChromeDeviceSettingsProto device_policy_proto; + em::AutoUpdateSettingsProto *auto_update_settings = + device_policy_proto.mutable_auto_update_settings(); + auto_update_settings->set_staging_schedule( + "[{\"days\": 4, \"percentage\": 40}, {\"days\": 10, \"percentage\": " + "100}]"); + InitializePolicy(InstallAttributesReader::kDeviceModeEnterpriseAD, + device_policy_proto); + + std::vector<DayPercentagePair> staging_schedule; + ASSERT_TRUE(device_policy_.GetDeviceUpdateStagingSchedule(&staging_schedule)); + EXPECT_THAT(staging_schedule, ElementsAre(DayPercentagePair{4, 40}, + DayPercentagePair{10, 100})); +} + +// Update staging schedule has values with values set larger than the max +// allowed days/percentage and smaller than the min allowed days/percentage. +TEST_F(DevicePolicyImplTest, + GetDeviceUpdateStagingSchedule_SetOutsideAllowable) { + em::ChromeDeviceSettingsProto device_policy_proto; + em::AutoUpdateSettingsProto *auto_update_settings = + device_policy_proto.mutable_auto_update_settings(); + auto_update_settings->set_staging_schedule( + "[{\"days\": -1, \"percentage\": -10}, {\"days\": 30, \"percentage\": " + "110}]"); + InitializePolicy(InstallAttributesReader::kDeviceModeEnterprise, + device_policy_proto); + + std::vector<DayPercentagePair> staging_schedule; + ASSERT_TRUE(device_policy_.GetDeviceUpdateStagingSchedule(&staging_schedule)); + EXPECT_THAT(staging_schedule, ElementsAre(DayPercentagePair{1, 0}, + DayPercentagePair{28, 100})); +} + +} // namespace policy diff --git a/policy/tests/libpolicy_unittest.cc b/policy/tests/libpolicy_unittest.cc index bd2b49a..aaf497c 100644 --- a/policy/tests/libpolicy_unittest.cc +++ b/policy/tests/libpolicy_unittest.cc @@ -4,6 +4,9 @@ #include "policy/libpolicy.h" +#include <memory> +#include <utility> + #include <openssl/err.h> #include <openssl/ssl.h> @@ -11,6 +14,8 @@ #include <base/logging.h> #include <gtest/gtest.h> +#include "bindings/chrome_device_policy.pb.h" +#include "install_attributes/mock_install_attributes_reader.h" #include "policy/device_policy_impl.h" namespace policy { @@ -18,6 +23,7 @@ namespace policy { static const char kPolicyFileAllSet[] = "policy/tests/whitelist/policy_all"; static const char kPolicyFileNoneSet[] = "policy/tests/whitelist/policy_none"; static const char kKeyFile[] = "policy/tests/whitelist/owner.key"; +static const char kNonExistingFile[] = "file-does-not-exist"; // Creates the DevicePolicyImpl with given parameters for test. std::unique_ptr<DevicePolicyImpl> CreateDevicePolicyImpl( @@ -40,10 +46,11 @@ std::unique_ptr<DevicePolicyImpl> CreateDevicePolicyImpl( TEST(PolicyTest, DevicePolicyAllSetTest) { base::FilePath policy_file(kPolicyFileAllSet); base::FilePath key_file(kKeyFile); - PolicyProvider provider( - CreateDevicePolicyImpl(std::make_unique<MockInstallAttributesReader>( - cryptohome::SerializedInstallAttributes()), - policy_file, key_file, false)); + PolicyProvider provider; + provider.SetDevicePolicyForTesting(CreateDevicePolicyImpl( + std::make_unique<MockInstallAttributesReader>( + InstallAttributesReader::kDeviceModeEnterprise, true), + policy_file, key_file, false)); provider.Reload(); // Ensure we successfully loaded the device policy file. @@ -54,111 +61,143 @@ TEST(PolicyTest, DevicePolicyAllSetTest) { // Check that we can read out all fields of the sample protobuf. int int_value = -1; ASSERT_TRUE(policy.GetPolicyRefreshRate(&int_value)); - ASSERT_EQ(100, int_value); + EXPECT_EQ(100, int_value); std::vector<std::string> list_value; ASSERT_TRUE(policy.GetUserWhitelist(&list_value)); ASSERT_EQ(3, list_value.size()); - ASSERT_EQ("me@here.com", list_value[0]); - ASSERT_EQ("you@there.com", list_value[1]); - ASSERT_EQ("*@monsters.com", list_value[2]); + EXPECT_EQ("me@here.com", list_value[0]); + EXPECT_EQ("you@there.com", list_value[1]); + EXPECT_EQ("*@monsters.com", list_value[2]); bool bool_value = true; ASSERT_TRUE(policy.GetGuestModeEnabled(&bool_value)); - ASSERT_FALSE(bool_value); + EXPECT_FALSE(bool_value); bool_value = true; ASSERT_TRUE(policy.GetCameraEnabled(&bool_value)); - ASSERT_FALSE(bool_value); + EXPECT_FALSE(bool_value); bool_value = true; ASSERT_TRUE(policy.GetShowUserNames(&bool_value)); - ASSERT_FALSE(bool_value); + EXPECT_FALSE(bool_value); bool_value = true; ASSERT_TRUE(policy.GetDataRoamingEnabled(&bool_value)); - ASSERT_FALSE(bool_value); + EXPECT_FALSE(bool_value); bool_value = true; ASSERT_TRUE(policy.GetAllowNewUsers(&bool_value)); - ASSERT_FALSE(bool_value); + EXPECT_FALSE(bool_value); bool_value = true; ASSERT_TRUE(policy.GetMetricsEnabled(&bool_value)); - ASSERT_FALSE(bool_value); + EXPECT_FALSE(bool_value); bool_value = true; ASSERT_TRUE(policy.GetReportVersionInfo(&bool_value)); - ASSERT_FALSE(bool_value); + EXPECT_FALSE(bool_value); bool_value = true; ASSERT_TRUE(policy.GetReportActivityTimes(&bool_value)); - ASSERT_FALSE(bool_value); + EXPECT_FALSE(bool_value); bool_value = true; ASSERT_TRUE(policy.GetReportBootMode(&bool_value)); - ASSERT_FALSE(bool_value); + EXPECT_FALSE(bool_value); bool_value = true; ASSERT_TRUE(policy.GetEphemeralUsersEnabled(&bool_value)); - ASSERT_FALSE(bool_value); + EXPECT_FALSE(bool_value); std::string string_value; ASSERT_TRUE(policy.GetReleaseChannel(&string_value)); - ASSERT_EQ("stable-channel", string_value); + EXPECT_EQ("stable-channel", string_value); bool_value = false; ASSERT_TRUE(policy.GetReleaseChannelDelegated(&bool_value)); - ASSERT_TRUE(bool_value); + EXPECT_TRUE(bool_value); bool_value = true; ASSERT_TRUE(policy.GetUpdateDisabled(&bool_value)); - ASSERT_FALSE(bool_value); + EXPECT_FALSE(bool_value); int64_t int64_value = -1LL; ASSERT_TRUE(policy.GetScatterFactorInSeconds(&int64_value)); - ASSERT_EQ(17LL, int64_value); + EXPECT_EQ(17LL, int64_value); ASSERT_TRUE(policy.GetTargetVersionPrefix(&string_value)); - ASSERT_EQ("42.0.", string_value); + EXPECT_EQ("42.0.", string_value); + + int_value = -1; + ASSERT_TRUE(policy.GetRollbackToTargetVersion(&int_value)); + EXPECT_EQ(enterprise_management::AutoUpdateSettingsProto:: + ROLLBACK_WITH_FULL_POWERWASH, + int_value); + + int_value = -1; + ASSERT_TRUE(policy.GetRollbackAllowedMilestones(&int_value)); + EXPECT_EQ(3, int_value); std::set<std::string> types; ASSERT_TRUE(policy.GetAllowedConnectionTypesForUpdate(&types)); - ASSERT_TRUE(types.end() != types.find("ethernet")); - ASSERT_TRUE(types.end() != types.find("wifi")); - ASSERT_EQ(2, types.size()); + EXPECT_TRUE(types.end() != types.find("ethernet")); + EXPECT_TRUE(types.end() != types.find("wifi")); + EXPECT_EQ(2, types.size()); ASSERT_TRUE(policy.GetOpenNetworkConfiguration(&string_value)); - ASSERT_EQ("{}", string_value); + EXPECT_EQ("{}", string_value); ASSERT_TRUE(policy.GetOwner(&string_value)); - ASSERT_EQ("", string_value); + EXPECT_EQ("", string_value); bool_value = true; ASSERT_TRUE(policy.GetHttpDownloadsEnabled(&bool_value)); - ASSERT_FALSE(bool_value); + EXPECT_FALSE(bool_value); bool_value = true; ASSERT_TRUE(policy.GetAuP2PEnabled(&bool_value)); - ASSERT_FALSE(bool_value); + EXPECT_FALSE(bool_value); bool_value = true; ASSERT_TRUE(policy.GetAllowKioskAppControlChromeVersion(&bool_value)); - ASSERT_FALSE(bool_value); + EXPECT_FALSE(bool_value); std::vector<DevicePolicy::UsbDeviceId> list_device; ASSERT_TRUE(policy.GetUsbDetachableWhitelist(&list_device)); - ASSERT_EQ(2, list_device.size()); - ASSERT_EQ(0x413c, list_device[0].vendor_id); - ASSERT_EQ(0x2105, list_device[0].product_id); - ASSERT_EQ(0x0403, list_device[1].vendor_id); - ASSERT_EQ(0x6001, list_device[1].product_id); + EXPECT_EQ(2, list_device.size()); + EXPECT_EQ(0x413c, list_device[0].vendor_id); + EXPECT_EQ(0x2105, list_device[0].product_id); + EXPECT_EQ(0x0403, list_device[1].vendor_id); + EXPECT_EQ(0x6001, list_device[1].product_id); + + ASSERT_TRUE(policy.GetAutoLaunchedKioskAppId(&string_value)); + EXPECT_EQ("my_kiosk_app", string_value); + + int_value = -1; + ASSERT_TRUE(policy.GetSecondFactorAuthenticationMode(&int_value)); + EXPECT_EQ(2, int_value); + + std::vector<DevicePolicy::WeeklyTimeInterval> intervals; + ASSERT_TRUE(policy.GetDisallowedTimeIntervals(&intervals)); + ASSERT_EQ(2, intervals.size()); + EXPECT_EQ(4, intervals[0].start_day_of_week); + EXPECT_EQ(base::TimeDelta::FromMinutes(30) + base::TimeDelta::FromHours(12), + intervals[0].start_time); + EXPECT_EQ(6, intervals[0].end_day_of_week); + EXPECT_EQ(base::TimeDelta::FromMinutes(15) + base::TimeDelta::FromHours(3), + intervals[0].end_time); + EXPECT_EQ(1, intervals[1].start_day_of_week); + EXPECT_EQ(base::TimeDelta::FromMinutes(10) + base::TimeDelta::FromHours(20), + intervals[1].start_time); + EXPECT_EQ(3, intervals[1].end_day_of_week); + EXPECT_EQ(base::TimeDelta::FromMinutes(20), intervals[1].end_time); ASSERT_TRUE(policy.GetAutoLaunchedKioskAppId(&string_value)); ASSERT_EQ("my_kiosk_app", string_value); // Reloading the protobuf should succeed. - ASSERT_TRUE(provider.Reload()); + EXPECT_TRUE(provider.Reload()); } // Test that a policy file can be verified and parsed correctly. The file @@ -167,10 +206,11 @@ TEST(PolicyTest, DevicePolicyNoneSetTest) { base::FilePath policy_file(kPolicyFileNoneSet); base::FilePath key_file(kKeyFile); - PolicyProvider provider( - CreateDevicePolicyImpl(std::make_unique<MockInstallAttributesReader>( - cryptohome::SerializedInstallAttributes()), - policy_file, key_file, false)); + PolicyProvider provider; + provider.SetDevicePolicyForTesting(CreateDevicePolicyImpl( + std::make_unique<MockInstallAttributesReader>( + InstallAttributesReader::kDeviceModeEnterprise, true), + policy_file, key_file, false)); provider.Reload(); // Ensure we successfully loaded the device policy file. @@ -185,45 +225,136 @@ TEST(PolicyTest, DevicePolicyNoneSetTest) { bool bool_value; std::string string_value; std::vector<DevicePolicy::UsbDeviceId> list_device; - - ASSERT_FALSE(policy.GetPolicyRefreshRate(&int_value)); - ASSERT_FALSE(policy.GetUserWhitelist(&list_value)); - ASSERT_FALSE(policy.GetGuestModeEnabled(&bool_value)); - ASSERT_FALSE(policy.GetCameraEnabled(&bool_value)); - ASSERT_FALSE(policy.GetShowUserNames(&bool_value)); - ASSERT_FALSE(policy.GetDataRoamingEnabled(&bool_value)); - ASSERT_FALSE(policy.GetAllowNewUsers(&bool_value)); - ASSERT_FALSE(policy.GetMetricsEnabled(&bool_value)); - ASSERT_FALSE(policy.GetReportVersionInfo(&bool_value)); - ASSERT_FALSE(policy.GetReportActivityTimes(&bool_value)); - ASSERT_FALSE(policy.GetReportBootMode(&bool_value)); - ASSERT_FALSE(policy.GetEphemeralUsersEnabled(&bool_value)); - ASSERT_FALSE(policy.GetReleaseChannel(&string_value)); - ASSERT_FALSE(policy.GetUpdateDisabled(&bool_value)); - ASSERT_FALSE(policy.GetTargetVersionPrefix(&string_value)); - ASSERT_FALSE(policy.GetScatterFactorInSeconds(&int64_value)); - ASSERT_FALSE(policy.GetOpenNetworkConfiguration(&string_value)); - ASSERT_FALSE(policy.GetHttpDownloadsEnabled(&bool_value)); - ASSERT_FALSE(policy.GetAuP2PEnabled(&bool_value)); - ASSERT_FALSE(policy.GetAllowKioskAppControlChromeVersion(&bool_value)); - ASSERT_FALSE(policy.GetUsbDetachableWhitelist(&list_device)); + std::vector<DevicePolicy::WeeklyTimeInterval> intervals; + + EXPECT_FALSE(policy.GetPolicyRefreshRate(&int_value)); + EXPECT_FALSE(policy.GetUserWhitelist(&list_value)); + EXPECT_FALSE(policy.GetGuestModeEnabled(&bool_value)); + EXPECT_FALSE(policy.GetCameraEnabled(&bool_value)); + EXPECT_FALSE(policy.GetShowUserNames(&bool_value)); + EXPECT_FALSE(policy.GetDataRoamingEnabled(&bool_value)); + EXPECT_FALSE(policy.GetAllowNewUsers(&bool_value)); + EXPECT_FALSE(policy.GetMetricsEnabled(&bool_value)); + EXPECT_FALSE(policy.GetReportVersionInfo(&bool_value)); + EXPECT_FALSE(policy.GetReportActivityTimes(&bool_value)); + EXPECT_FALSE(policy.GetReportBootMode(&bool_value)); + EXPECT_FALSE(policy.GetEphemeralUsersEnabled(&bool_value)); + EXPECT_FALSE(policy.GetReleaseChannel(&string_value)); + EXPECT_FALSE(policy.GetUpdateDisabled(&bool_value)); + EXPECT_FALSE(policy.GetTargetVersionPrefix(&string_value)); + EXPECT_FALSE(policy.GetRollbackToTargetVersion(&int_value)); + // RollbackAllowedMilestones has the default value of 0 for enterprise + // devices. + ASSERT_TRUE(policy.GetRollbackAllowedMilestones(&int_value)); + EXPECT_EQ(0, int_value); + EXPECT_FALSE(policy.GetScatterFactorInSeconds(&int64_value)); + EXPECT_FALSE(policy.GetOpenNetworkConfiguration(&string_value)); + EXPECT_FALSE(policy.GetHttpDownloadsEnabled(&bool_value)); + EXPECT_FALSE(policy.GetAuP2PEnabled(&bool_value)); + EXPECT_FALSE(policy.GetAllowKioskAppControlChromeVersion(&bool_value)); + EXPECT_FALSE(policy.GetUsbDetachableWhitelist(&list_device)); + EXPECT_FALSE(policy.GetSecondFactorAuthenticationMode(&int_value)); + EXPECT_FALSE(policy.GetDisallowedTimeIntervals(&intervals)); } // Verify that the library will correctly recognize and signal missing files. TEST(PolicyTest, DevicePolicyFailure) { LOG(INFO) << "Errors expected."; // Try loading non-existing protobuf should fail. - base::FilePath non_existing("this_file_is_doof"); - PolicyProvider provider( + base::FilePath policy_file(kNonExistingFile); + base::FilePath key_file(kNonExistingFile); + PolicyProvider provider; + provider.SetDevicePolicyForTesting( CreateDevicePolicyImpl(std::make_unique<MockInstallAttributesReader>( cryptohome::SerializedInstallAttributes()), - non_existing, - non_existing, - true)); + policy_file, key_file, true)); // Even after reload the policy should still be not loaded. ASSERT_FALSE(provider.Reload()); - ASSERT_FALSE(provider.device_policy_is_loaded()); + EXPECT_FALSE(provider.device_policy_is_loaded()); +} + +// Verify that signature verification is waived for a device in enterprise_ad +// mode. +TEST(PolicyTest, SkipSignatureForEnterpriseAD) { + base::FilePath policy_file(kPolicyFileAllSet); + base::FilePath key_file(kNonExistingFile); + PolicyProvider provider; + provider.SetDevicePolicyForTesting(CreateDevicePolicyImpl( + std::make_unique<MockInstallAttributesReader>( + InstallAttributesReader::kDeviceModeEnterpriseAD, true), + policy_file, key_file, false)); + provider.Reload(); + + // Ensure we successfully loaded the device policy file. + EXPECT_TRUE(provider.device_policy_is_loaded()); +} + +// Ensure that signature verification is enforced for a device in vanilla +// enterprise mode. +TEST(PolicyTest, DontSkipSignatureForEnterprise) { + base::FilePath policy_file(kPolicyFileAllSet); + base::FilePath key_file(kNonExistingFile); + + PolicyProvider provider; + provider.SetDevicePolicyForTesting(CreateDevicePolicyImpl( + std::make_unique<MockInstallAttributesReader>( + InstallAttributesReader::kDeviceModeEnterprise, true), + policy_file, key_file, false)); + provider.Reload(); + + // Ensure that unverifed policy is not loaded. + EXPECT_FALSE(provider.device_policy_is_loaded()); +} + +// Ensure that signature verification is enforced for a device in consumer mode. +TEST(PolicyTest, DontSkipSignatureForConsumer) { + base::FilePath policy_file(kPolicyFileAllSet); + base::FilePath key_file(kNonExistingFile); + cryptohome::SerializedInstallAttributes install_attributes; + + PolicyProvider provider; + provider.SetDevicePolicyForTesting(CreateDevicePolicyImpl( + std::make_unique<MockInstallAttributesReader>(install_attributes), + policy_file, key_file, false)); + provider.Reload(); + + // Ensure that unverifed policy is not loaded. + EXPECT_FALSE(provider.device_policy_is_loaded()); +} + +// Checks return value of IsConsumerDevice when it's a still in OOBE. +TEST(PolicyTest, IsConsumerDeviceOobe) { + PolicyProvider provider; + provider.SetInstallAttributesReaderForTesting( + std::make_unique<MockInstallAttributesReader>("", false)); + EXPECT_FALSE(provider.IsConsumerDevice()); +} + +// Checks return value of IsConsumerDevice when it's a consumer device. +TEST(PolicyTest, IsConsumerDeviceConsumer) { + PolicyProvider provider; + provider.SetInstallAttributesReaderForTesting( + std::make_unique<MockInstallAttributesReader>("", true)); + EXPECT_TRUE(provider.IsConsumerDevice()); +} + +// Checks return value of IsConsumerDevice when it's an enterprise device. +TEST(PolicyTest, IsConsumerDeviceEnterprise) { + PolicyProvider provider; + provider.SetInstallAttributesReaderForTesting( + std::make_unique<MockInstallAttributesReader>( + InstallAttributesReader::kDeviceModeEnterprise, true)); + EXPECT_FALSE(provider.IsConsumerDevice()); +} + +// Checks return value of IsConsumerDevice when it's an enterprise AD device. +TEST(PolicyTest, IsConsumerDeviceEnterpriseAd) { + PolicyProvider provider; + provider.SetInstallAttributesReaderForTesting( + std::make_unique<MockInstallAttributesReader>( + InstallAttributesReader::kDeviceModeEnterpriseAD, true)); + EXPECT_FALSE(provider.IsConsumerDevice()); } } // namespace policy diff --git a/policy/tests/policy_util_unittest.cc b/policy/tests/policy_util_unittest.cc index 74032d4..f26622f 100644 --- a/policy/tests/policy_util_unittest.cc +++ b/policy/tests/policy_util_unittest.cc @@ -20,9 +20,9 @@ TEST(DevicePolicyUtilTest, LoadPolicyFromPath) { base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); - base::FilePath invalid_policy_data_path(temp_dir.path().Append("policy")); - base::FilePath inexistent_file(temp_dir.path().Append("policy.1")); - base::FilePath good_policy_data_path(temp_dir.path().Append("policy.2")); + base::FilePath invalid_policy_data_path(temp_dir.GetPath().Append("policy")); + base::FilePath inexistent_file(temp_dir.GetPath().Append("policy.1")); + base::FilePath good_policy_data_path(temp_dir.GetPath().Append("policy.2")); // Create the file with invalid data. std::string data = "invalid data"; diff --git a/policy/tests/resilient_policy_util_unittest.cc b/policy/tests/resilient_policy_util_unittest.cc index e45ab34..0963b08 100644 --- a/policy/tests/resilient_policy_util_unittest.cc +++ b/policy/tests/resilient_policy_util_unittest.cc @@ -31,11 +31,11 @@ TEST(DevicePolicyUtilTest, GetSortedResilientPolicyFilePaths) { base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); - base::FilePath file0(temp_dir.path().Append("policy")); - base::FilePath file1(temp_dir.path().Append("policy.12")); - base::FilePath file2(temp_dir.path().Append("policy.2")); - base::FilePath file3(temp_dir.path().Append("policy.30")); - base::FilePath invalid(temp_dir.path().Append("policy_4")); + base::FilePath file0(temp_dir.GetPath().Append("policy")); + base::FilePath file1(temp_dir.GetPath().Append("policy.12")); + base::FilePath file2(temp_dir.GetPath().Append("policy.2")); + base::FilePath file3(temp_dir.GetPath().Append("policy.30")); + base::FilePath invalid(temp_dir.GetPath().Append("policy_4")); CreateFile(file0); CreateFile(file1); @@ -43,7 +43,7 @@ TEST(DevicePolicyUtilTest, GetSortedResilientPolicyFilePaths) { CreateFile(file3); const base::FilePath test_file_path( - temp_dir.path().Append(kDefaultResilientPolicyFilePath)); + temp_dir.GetPath().Append(kDefaultResilientPolicyFilePath)); std::map<int, base::FilePath> sorted_file_paths = GetSortedResilientPolicyFilePaths(test_file_path); diff --git a/policy/tests/whitelist/policy_all b/policy/tests/whitelist/policy_all Binary files differindex ff197cd..c0dfaaf 100644 --- a/policy/tests/whitelist/policy_all +++ b/policy/tests/whitelist/policy_all diff --git a/testrunner.cc b/testrunner.cc index 7122b9c..4a03685 100644 --- a/testrunner.cc +++ b/testrunner.cc @@ -4,7 +4,6 @@ // based on pam_google_testrunner.cc -#include <glib-object.h> #include <gtest/gtest.h> #include <base/at_exit.h> @@ -12,7 +11,6 @@ int main(int argc, char** argv) { base::AtExitManager at_exit_manager; - ::g_type_init(); SetUpTests(&argc, argv, true); return RUN_ALL_TESTS(); } |