From f6f9480874fdec3fe724e9c8f1ce519bc0dd80fb Mon Sep 17 00:00:00 2001 From: Teddy Reed Date: Fri, 24 Mar 2017 18:47:23 -0700 Subject: [PATCH] extensions: Preserve environment in auto-loaded extensions (#3101) --- osquery/core/posix/process.cpp | 4 +-- osquery/core/process.h | 2 +- osquery/core/testing.h | 43 ---------------------------- osquery/core/tests/process_tests.cpp | 42 +++++++++++++-------------- osquery/core/tests/watcher_tests.cpp | 2 +- osquery/core/watcher.cpp | 2 +- osquery/core/windows/process.cpp | 7 ++--- osquery/main/tests.cpp | 3 +- osquery/tests/test_util.h | 31 ++++++++++++++++++-- 9 files changed, 57 insertions(+), 79 deletions(-) delete mode 100644 osquery/core/testing.h diff --git a/osquery/core/posix/process.cpp b/osquery/core/posix/process.cpp index 2205553e..2cb16c55 100644 --- a/osquery/core/posix/process.cpp +++ b/osquery/core/posix/process.cpp @@ -117,7 +117,7 @@ std::shared_ptr PlatformProcess::launchExtension( const std::string& extensions_socket, const std::string& extensions_timeout, const std::string& extensions_interval, - const std::string& verbose) { + bool verbose) { auto ext_pid = ::fork(); if (ext_pid < 0) { return std::shared_ptr(); @@ -125,13 +125,13 @@ std::shared_ptr PlatformProcess::launchExtension( setEnvVar("OSQUERY_EXTENSION", std::to_string(::getpid()).c_str()); ::execle(exec_path.c_str(), ("osquery extension: " + extension).c_str(), + (verbose) ? "--verbose" : "--noverbose", "--socket", extensions_socket.c_str(), "--timeout", extensions_timeout.c_str(), "--interval", extensions_interval.c_str(), - (verbose == "true") ? "--verbose" : (char*)nullptr, (char*)nullptr, ::environ); diff --git a/osquery/core/process.h b/osquery/core/process.h index 667e7930..855fc649 100644 --- a/osquery/core/process.h +++ b/osquery/core/process.h @@ -162,7 +162,7 @@ class PlatformProcess : private boost::noncopyable { const std::string& extensions_socket, const std::string& extensions_timeout, const std::string& extensions_interval, - const std::string& verbose); + bool verbose = false); /** * @brief Launches a new Python script diff --git a/osquery/core/testing.h b/osquery/core/testing.h deleted file mode 100644 index be5aedc0..00000000 --- a/osquery/core/testing.h +++ /dev/null @@ -1,43 +0,0 @@ -/* -* Copyright (c) 2014-present, Facebook, Inc. -* All rights reserved. -* -* This source code is licensed under the BSD-style license found in the -* LICENSE file in the root directory of this source tree. An additional grant -* of patent rights can be found in the PATENTS file in the same directory. -* -*/ - -#pragma once - -/// The following codes are specifically for checking whether the child worker -/// or extension process ran successfully. These values should be the values -/// captured as exit codes if all the child process checks complete without -/// deviation. -#define EXTENSION_SUCCESS_CODE 0x45 -#define WORKER_SUCCESS_CODE 0x57 - -/// The following are error codes returned by the child process. -#define ERROR_COMPARE_ARGUMENT -1 -#define ERROR_LAUNCHER_PROCESS -2 -#define ERROR_QUERY_PROCESS_IMAGE -3 -#define ERROR_IMAGE_NAME_LENGTH -4 -#define ERROR_LAUNCHER_MISMATCH -5 - -namespace osquery { - -/// Stores the path of the currently executing executable -extern std::string kProcessTestExecPath; - -/// This is the expected module name of the launcher process. -extern const char *kOsqueryTestModuleName; - -/// These are the expected arguments for our test worker process. -extern const char *kExpectedWorkerArgs[]; -extern const size_t kExpectedWorkerArgsCount; - -/// These are the expected arguments for our test extensions process. -extern const char *kExpectedExtensionArgs[]; -extern const size_t kExpectedExtensionArgsCount; -} - diff --git a/osquery/core/tests/process_tests.cpp b/osquery/core/tests/process_tests.cpp index 1f4d31da..76f4d83c 100644 --- a/osquery/core/tests/process_tests.cpp +++ b/osquery/core/tests/process_tests.cpp @@ -13,7 +13,7 @@ #include #include "osquery/core/process.h" -#include "osquery/core/testing.h" +#include "osquery/tests/test_util.h" namespace osquery { @@ -89,8 +89,7 @@ TEST_F(ProcessTests, test_constructorPosix) { TEST_F(ProcessTests, test_getpid) { int pid = -1; - std::shared_ptr process = - PlatformProcess::getCurrentProcess(); + auto process = PlatformProcess::getCurrentProcess(); EXPECT_NE(nullptr, process.get()); #ifdef WIN32 @@ -123,13 +122,13 @@ TEST_F(ProcessTests, test_envVar) { TEST_F(ProcessTests, test_launchExtension) { { - std::shared_ptr process = - osquery::PlatformProcess::launchExtension(kProcessTestExecPath.c_str(), - "extension-test", - kExpectedExtensionArgs[2], - kExpectedExtensionArgs[4], - kExpectedExtensionArgs[6], - "true"); + auto process = + PlatformProcess::launchExtension(kProcessTestExecPath.c_str(), + "extension-test", + kExpectedExtensionArgs[3], + kExpectedExtensionArgs[5], + kExpectedExtensionArgs[7], + true); EXPECT_NE(nullptr, process.get()); int code = 0; @@ -150,11 +149,10 @@ TEST_F(ProcessTests, test_launchWorker) { } argv.push_back(nullptr); - std::shared_ptr process = - osquery::PlatformProcess::launchWorker( - kProcessTestExecPath.c_str(), - static_cast(kExpectedWorkerArgsCount), - &argv[0]); + auto process = PlatformProcess::launchWorker( + kProcessTestExecPath.c_str(), + static_cast(kExpectedWorkerArgsCount), + &argv[0]); for (size_t i = 0; i < argv.size(); i++) { delete[] argv[i]; } @@ -170,13 +168,13 @@ TEST_F(ProcessTests, test_launchWorker) { #ifdef WIN32 TEST_F(ProcessTests, test_launchExtensionQuotes) { { - std::shared_ptr process = - osquery::PlatformProcess::launchExtension(kProcessTestExecPath.c_str(), - "exten\"sion-te\"st", - "socket-name", - "100", - "5", - "true"); + auto process = + PlatformProcess::launchExtension(kProcessTestExecPath.c_str(), + "exten\"sion-te\"st", + "socket-name", + "100", + "5", + true); EXPECT_NE(nullptr, process.get()); int code = 0; diff --git a/osquery/core/tests/watcher_tests.cpp b/osquery/core/tests/watcher_tests.cpp index 6d9b63cd..dfcde092 100644 --- a/osquery/core/tests/watcher_tests.cpp +++ b/osquery/core/tests/watcher_tests.cpp @@ -14,8 +14,8 @@ #include #include -#include "osquery/core/testing.h" #include "osquery/core/watcher.h" +#include "osquery/tests/test_util.h" using namespace testing; diff --git a/osquery/core/watcher.cpp b/osquery/core/watcher.cpp index c129db6b..54511933 100644 --- a/osquery/core/watcher.cpp +++ b/osquery/core/watcher.cpp @@ -493,7 +493,7 @@ void WatcherRunner::createExtension(const std::string& extension) { Flag::getValue("extensions_socket"), Flag::getValue("extensions_timeout"), Flag::getValue("extensions_interval"), - Flag::getValue("verbose")); + Flag::getValue("verbose") == "true"); if (ext_process == nullptr) { // Unrecoverable error, cannot create an extension process. LOG(ERROR) << "Cannot create extension process: " << extension; diff --git a/osquery/core/windows/process.cpp b/osquery/core/windows/process.cpp index ef4323ce..5c1a2ad0 100644 --- a/osquery/core/windows/process.cpp +++ b/osquery/core/windows/process.cpp @@ -242,7 +242,7 @@ std::shared_ptr PlatformProcess::launchExtension( const std::string& extensions_socket, const std::string& extensions_timeout, const std::string& extensions_interval, - const std::string& verbose) { + bool verbose) { ::STARTUPINFOA si = {0}; ::PROCESS_INFORMATION pi = {nullptr}; @@ -253,14 +253,11 @@ std::shared_ptr PlatformProcess::launchExtension( std::stringstream argv_stream; argv_stream << "\"osquery extension: " << boost::replace_all_copy(extension, "\"", "") << "\" "; + argv_stream << ((verbose) ? "--verbose" : "--noverbose") << " "; argv_stream << "--socket \"" << extensions_socket << "\" "; argv_stream << "--timeout " << extensions_timeout << " "; argv_stream << "--interval " << extensions_interval << " "; - if (verbose == "true") { - argv_stream << "--verbose"; - } - // We don't directly use argv.c_str() as the value for lpCommandLine in // CreateProcess since that argument requires a modifiable buffer. So, // instead, we off-load the contents of argv into a vector which will have its diff --git a/osquery/main/tests.cpp b/osquery/main/tests.cpp index 8b2812d6..86fb7ba4 100644 --- a/osquery/main/tests.cpp +++ b/osquery/main/tests.cpp @@ -21,7 +21,6 @@ #include #include "osquery/core/process.h" -#include "osquery/core/testing.h" #include "osquery/tests/test_util.h" namespace osquery { @@ -42,13 +41,13 @@ const size_t kExpectedWorkerArgsCount = /// These are the expected arguments for our test extensions process. const char* kExpectedExtensionArgs[] = {"osquery extension: extension-test", + "--verbose", "--socket", "socket-name", "--timeout", "100", "--interval", "5", - "--verbose", nullptr}; const size_t kExpectedExtensionArgsCount = (sizeof(osquery::kExpectedExtensionArgs) / sizeof(char*)) - 1; diff --git a/osquery/tests/test_util.h b/osquery/tests/test_util.h index 71930c6c..fd7f7cd2 100644 --- a/osquery/tests/test_util.h +++ b/osquery/tests/test_util.h @@ -25,6 +25,20 @@ namespace pt = boost::property_tree; namespace osquery { +/// The following codes are specifically for checking whether the child worker +/// or extension process ran successfully. These values should be the values +/// captured as exit codes if all the child process checks complete without +/// deviation. +#define EXTENSION_SUCCESS_CODE 0x45 +#define WORKER_SUCCESS_CODE 0x57 + +/// The following are error codes returned by the child process. +#define ERROR_COMPARE_ARGUMENT -1 +#define ERROR_LAUNCHER_PROCESS -2 +#define ERROR_QUERY_PROCESS_IMAGE -3 +#define ERROR_IMAGE_NAME_LENGTH -4 +#define ERROR_LAUNCHER_MISMATCH -5 + /// Init function for tests and benchmarks. void initTesting(); @@ -46,6 +60,20 @@ extern std::string kTestDataPath; extern std::string kTestWorkingDirectory; extern std::string kFakeDirectory; +/// Stores the path of the currently executing executable +extern std::string kProcessTestExecPath; + +/// This is the expected module name of the launcher process. +extern const char* kOsqueryTestModuleName; + +/// These are the expected arguments for our test worker process. +extern const char* kExpectedWorkerArgs[]; +extern const size_t kExpectedWorkerArgsCount; + +/// These are the expected arguments for our test extensions process. +extern const char* kExpectedExtensionArgs[]; +extern const size_t kExpectedExtensionArgsCount; + // Get an example generate config with one static source name to JSON content. std::map getTestConfigMap(); @@ -65,7 +93,7 @@ QueryData getTestDBExpectedResults(); // Starting with the dataset returned by createTestDB(), getTestDBResultStream // returns a vector of std::pair's where pair.first is the query that would // need to be performed on the dataset to make the results be pair.second -std::vector > getTestDBResultStream(); +std::vector> getTestDBResultStream(); // getSerializedRowColumnNames returns a vector of test column names that // are in alphabetical order. If unordered_and_repeated is true, the @@ -129,4 +157,3 @@ void createMockFileStructure(); // remove the small directory structure used for testing void tearDownMockFileStructure(); } -