From 2bd7e8660881b2863811a04e34f8ff3d2d748a3d Mon Sep 17 00:00:00 2001 From: Zach Wasserman Date: Fri, 9 Feb 2024 10:47:09 -0800 Subject: [PATCH] Refactor macOS system profile report retrieval (#8251) Move the report retrieval into a utility file and then refactor the two current tables that use it (`connected_displays` and `secureboot`) to use the new helper. Tested that the tables continue to work on both Intel and ARM64. --- osquery/tables/applications/CMakeLists.txt | 1 + osquery/tables/system/CMakeLists.txt | 1 + .../system/darwin/connected_displays.mm | 76 ++------------- osquery/tables/system/darwin/secureboot.mm | 68 ++------------ osquery/tables/system/tests/CMakeLists.txt | 20 ++-- osquery/utils/CMakeLists.txt | 29 +++++- osquery/utils/darwin/system_profiler.h | 33 +++++++ osquery/utils/darwin/system_profiler.mm | 94 +++++++++++++++++++ osquery/utils/tests/darwin/system_profiler.mm | 25 +++++ 9 files changed, 204 insertions(+), 143 deletions(-) create mode 100644 osquery/utils/darwin/system_profiler.h create mode 100644 osquery/utils/darwin/system_profiler.mm create mode 100644 osquery/utils/tests/darwin/system_profiler.mm diff --git a/osquery/tables/applications/CMakeLists.txt b/osquery/tables/applications/CMakeLists.txt index bb6f4d3703b..1fd3d0e93aa 100644 --- a/osquery/tables/applications/CMakeLists.txt +++ b/osquery/tables/applications/CMakeLists.txt @@ -67,6 +67,7 @@ function(generateOsqueryTablesApplications) if(DEFINED PLATFORM_MACOS) target_link_libraries(osquery_tables_applications PRIVATE osquery_utils_plist + osquery_utils_system_profiler ) endif() diff --git a/osquery/tables/system/CMakeLists.txt b/osquery/tables/system/CMakeLists.txt index d6c2fdcfe40..5bfde7af2b0 100644 --- a/osquery/tables/system/CMakeLists.txt +++ b/osquery/tables/system/CMakeLists.txt @@ -293,6 +293,7 @@ function(generateOsqueryTablesSystemSystemtable) elseif(DEFINED PLATFORM_MACOS) target_link_libraries(osquery_tables_system_systemtable PUBLIC osquery_utils_plist + osquery_utils_system_profiler thirdparty_openssl ) diff --git a/osquery/tables/system/darwin/connected_displays.mm b/osquery/tables/system/darwin/connected_displays.mm index d9d67978613..b87272fe76f 100644 --- a/osquery/tables/system/darwin/connected_displays.mm +++ b/osquery/tables/system/darwin/connected_displays.mm @@ -12,11 +12,7 @@ #include #include - -@interface SPDocument : NSDocument { -} -- (id)reportForDataType:(id)arg1; -@end +#include namespace osquery { namespace tables { @@ -26,72 +22,14 @@ QueryData genConnectedDisplays(QueryContext& context) { @autoreleasepool { Row r; - // BEWARE: Because of the dynamic nature of the calls in this function, we - // must be careful to properly clean up the memory. Any future modifications - // to this function should attempt to ensure there are no leaks. - CFURLRef bundle_url = CFURLCreateWithFileSystemPath( - kCFAllocatorDefault, - CFSTR("/System/Library/PrivateFrameworks/SPSupport.framework"), - kCFURLPOSIXPathStyle, - true); - - if (bundle_url == nullptr) { - LOG(INFO) << "Error parsing SPSupport bundle URL"; - return results; - } - - CFBundleRef bundle = CFBundleCreate(kCFAllocatorDefault, bundle_url); - CFRelease(bundle_url); - if (bundle == nullptr) { - LOG(INFO) << "Error opening SPSupport bundle"; - return results; - } - - CFBundleLoadExecutable(bundle); - - std::function cleanup = [&]() { - CFBundleUnloadExecutable(bundle); - CFRelease(bundle); - }; - -#pragma clang diagnostic push -// We are silencing here because we don't know the selector beforehand -#pragma clang diagnostic ignored "-Warc-performSelector-leaks" - - id cls = NSClassFromString(@"SPDocument"); - if (cls == nullptr) { - LOG(INFO) << "Could not load SPDocument class"; - cleanup(); - + NSDictionary* __autoreleasing result; + Status status = getSystemProfilerReport("SPDisplaysDataType", result); + if (!status.ok()) { + LOG(ERROR) << "Failed to get connected displays: " << status.getMessage(); return results; } - SEL sel = @selector(new); - if (![cls respondsToSelector:sel]) { - LOG(INFO) << "SPDocument does not respond to new selector"; - cleanup(); - - return results; - } - - id document = [cls performSelector:sel]; - if (document == nullptr) { - LOG(INFO) << "[SPDocument new] returned null"; - cleanup(); - - return results; - } - -#pragma clang diagnostic pop - - cleanup = [&]() { - CFRelease((__bridge CFTypeRef)document); - CFBundleUnloadExecutable(bundle); - CFRelease(bundle); - }; - - NSDictionary* report = [[[document reportForDataType:@"SPDisplaysDataType"] - objectForKey:@"_items"] lastObject]; + NSDictionary* report = [[result objectForKey:@"_items"] lastObject]; NSArray* data = [report valueForKeyPath:@"spdisplays_ndrvs"]; for (NSString* obj in data) { @@ -207,8 +145,6 @@ QueryData genConnectedDisplays(QueryContext& context) { results.push_back(r); } - - cleanup(); } return results; } // context diff --git a/osquery/tables/system/darwin/secureboot.mm b/osquery/tables/system/darwin/secureboot.mm index b27ce589f70..4aee2b9e464 100644 --- a/osquery/tables/system/darwin/secureboot.mm +++ b/osquery/tables/system/darwin/secureboot.mm @@ -12,20 +12,13 @@ #include #include #include -#include +#include -#import #include -#import #include #include #include -@interface SPDocument : NSDocument { -} -- (id)reportForDataType:(id)arg1; -@end - namespace osquery::tables { namespace { @@ -186,61 +179,14 @@ Status getIntelSecureBootSetting(Row& row) { Status getAarch64SecureBootSetting(Row& r) { @autoreleasepool { - // BEWARE: Because of the dynamic nature of the calls in this function, we - // must be careful to properly clean up the memory. Any future modifications - // to this function should attempt to ensure there are no leaks (and test - // with ./tools/analysis/profile.py --leaks). - CFURLRef bundle_url = CFURLCreateWithFileSystemPath( - kCFAllocatorDefault, - CFSTR("/System/Library/PrivateFrameworks/SPSupport.framework"), - kCFURLPOSIXPathStyle, - true); - - if (bundle_url == nullptr) { - return Status::failure("Error parsing SPSupport bundle URL"); - } - - CFBundleRef bundle = CFBundleCreate(kCFAllocatorDefault, bundle_url); - CFRelease(bundle_url); - if (bundle == nullptr) { - return Status::failure("Error opening SPSupport bundle"); - } - - auto cleanup_bundle = scope_guard::create([&]() { - CFBundleUnloadExecutable(bundle); - CFRelease(bundle); - }); - - if (!CFBundleLoadExecutable(bundle)) { - return Status::failure("SPSupport load executable failed"); - } - -#pragma clang diagnostic push -// We are silencing here because we don't know the selector beforehand -#pragma clang diagnostic ignored "-Warc-performSelector-leaks" - - id cls = NSClassFromString(@"SPDocument"); - if (cls == nullptr) { - return Status::failure("Could not load SPDocument class"); - } - - SEL sel = @selector(new); - if (![cls respondsToSelector:sel]) { - return Status::failure("SPDocument does not respond to new selector"); + NSDictionary* __autoreleasing result; + Status status = getSystemProfilerReport("SPiBridgeDataType", result); + if (!status.ok()) { + return Status::failure("failed to get secureboot config: " + + status.getMessage()); } - id document = [cls performSelector:sel]; - if (document == nullptr) { - return Status::failure("[SPDocument new] returned null"); - } - - auto cleanup_document = - scope_guard::create([&]() { CFRelease((__bridge CFTypeRef)document); }); - -#pragma clang diagnostic pop - - NSDictionary* report = [[[document reportForDataType:@"SPiBridgeDataType"] - objectForKey:@"_items"] lastObject]; + NSDictionary* report = [[result objectForKey:@"_items"] lastObject]; if ([report valueForKey:@"ibridge_secure_boot"]) { r["description"] = diff --git a/osquery/tables/system/tests/CMakeLists.txt b/osquery/tables/system/tests/CMakeLists.txt index a2890f643b2..7e9fae12f5f 100644 --- a/osquery/tables/system/tests/CMakeLists.txt +++ b/osquery/tables/system/tests/CMakeLists.txt @@ -6,7 +6,6 @@ # SPDX-License-Identifier: (Apache-2.0 OR GPL-2.0-only) function(osqueryTablesSystemTestsMain) - if(DEFINED PLATFORM_LINUX) generateOsqueryTablesSystemLinuxTests() endif() @@ -136,6 +135,7 @@ function(generateOsqueryTablesSystemDarwinTests) osquery_utils osquery_utils_conversions osquery_utils_plist + osquery_utils_system_profiler tests_helper thirdparty_googletest ) @@ -169,20 +169,20 @@ endfunction() function(generateOsqueryTablesSystemDarwinKeychainTests) set(source_files - darwin/keychain_test.cpp + darwin/keychain_test.cpp ) add_osquery_executable(osquery_tables_system_darwin_keychain_tests-test ${source_files}) target_link_libraries(osquery_tables_system_darwin_keychain_tests-test PRIVATE - osquery_cxx_settings - osquery_filesystem - osquery_hashing - osquery_tables_system_systemtable - osquery_extensions - osquery_extensions_implthrift - tests_helper - thirdparty_googletest + osquery_cxx_settings + osquery_filesystem + osquery_hashing + osquery_tables_system_systemtable + osquery_extensions + osquery_extensions_implthrift + tests_helper + thirdparty_googletest ) endfunction() diff --git a/osquery/utils/CMakeLists.txt b/osquery/utils/CMakeLists.txt index 12c1c20e16a..48f934de69b 100644 --- a/osquery/utils/CMakeLists.txt +++ b/osquery/utils/CMakeLists.txt @@ -35,8 +35,10 @@ function(osqueryUtilsMain) endif() generateOsqueryUtils() + if(DEFINED PLATFORM_MACOS) generateOsqueryUtilsPlist() + generateOsqueryUtilsSystemProfiler() endif() endfunction() @@ -111,11 +113,9 @@ function(generateOsqueryUtils) generateIncludeNamespace(osquery_utils_attribute "osquery/utils" "FILE_ONLY" ${attribute_public_header_files}) add_test(NAME osquery_utils_utilstests-test COMMAND osquery_utils_utilstests-test) - endfunction() function(generateOsqueryUtilsPlist) - list(APPEND source_files darwin/plist.mm ) @@ -133,7 +133,26 @@ function(generateOsqueryUtilsPlist) ) generateIncludeNamespace(osquery_utils "osquery/utils" "FULL_PATH" ${platform_public_header_files}) +endfunction() +function(generateOsqueryUtilsSystemProfiler) + list(APPEND source_files + darwin/system_profiler.mm + ) + + add_osquery_library(osquery_utils_system_profiler ${source_files}) + + target_link_libraries(osquery_utils_system_profiler PRIVATE + osquery_cxx_settings + osquery_filesystem + thirdparty_boost + ) + + set(platform_public_header_files + darwin/system_profiler.h + ) + + generateIncludeNamespace(osquery_utils "osquery/utils" "FULL_PATH" ${platform_public_header_files}) endfunction() function(generateOsqueryUtilsUtilstestsTest) @@ -153,6 +172,12 @@ function(generateOsqueryUtilsUtilstestsTest) ) endif() + if(DEFINED PLATFORM_DARWIN) + list(APPEND source_files + tests/darwin/system_profiler.mm + ) + endif() + add_osquery_executable(osquery_utils_utilstests-test ${source_files}) target_link_libraries(osquery_utils_utilstests-test PRIVATE diff --git a/osquery/utils/darwin/system_profiler.h b/osquery/utils/darwin/system_profiler.h new file mode 100644 index 00000000000..6efa1d7a2e9 --- /dev/null +++ b/osquery/utils/darwin/system_profiler.h @@ -0,0 +1,33 @@ +/** + * Copyright (c) 2014-present, The osquery authors + * + * This source code is licensed as defined by the LICENSE file found in the + * root directory of this source tree. + * + * SPDX-License-Identifier: (Apache-2.0 OR GPL-2.0-only) + */ + +#pragma once + +#include + +#import + +namespace osquery { + +/** + * @brief Retrieve data from the macOS System Profiler/System Information + * utility. + * + * This could be called from within an @autoreleasepool. + * + * @param datatype the data type to request (see `system_profiler + * -listDataTypes`). + * @param result the NSDictionary pointer for returning results. + * + * @return an instance of Status, indicating success or failure. + */ +Status getSystemProfilerReport(const std::string& datatype, + NSDictionary*& result); + +} // namespace osquery diff --git a/osquery/utils/darwin/system_profiler.mm b/osquery/utils/darwin/system_profiler.mm new file mode 100644 index 00000000000..1702c5ba9c9 --- /dev/null +++ b/osquery/utils/darwin/system_profiler.mm @@ -0,0 +1,94 @@ +/** + * Copyright (c) 2014-present, The osquery authors + * + * This source code is licensed as defined by the LICENSE file found in the + * root directory of this source tree. + * + * SPDX-License-Identifier: (Apache-2.0 OR GPL-2.0-only) + */ + +#include "system_profiler.h" + +#include + +#include +#include +#include +#include + +#import +#include +#import +#include +#include +#include + +@interface SPDocument : NSDocument { +} +- (NSMutableDictionary*)reportForDataType:(NSString*)datatype; +@end + +namespace osquery { + +Status getSystemProfilerReport(const std::string& datatype, + NSDictionary*& result) { + // BEWARE: Because of the dynamic nature of the calls in this function, we + // must be careful to properly clean up the memory. Any future modifications + // to this function should attempt to ensure there are no leaks (and test + // with ./tools/analysis/profile.py --leaks). + CFURLRef bundle_url = CFURLCreateWithFileSystemPath( + kCFAllocatorDefault, + CFSTR("/System/Library/PrivateFrameworks/SPSupport.framework"), + kCFURLPOSIXPathStyle, + true); + + if (bundle_url == nullptr) { + return Status::failure("Error parsing SPSupport bundle URL"); + } + + CFBundleRef bundle = CFBundleCreate(kCFAllocatorDefault, bundle_url); + CFRelease(bundle_url); + if (bundle == nullptr) { + return Status::failure("Error opening SPSupport bundle"); + } + + auto cleanup_bundle = scope_guard::create([&]() { + CFBundleUnloadExecutable(bundle); + CFRelease(bundle); + }); + + if (!CFBundleLoadExecutable(bundle)) { + return Status::failure("SPSupport load executable failed"); + } + +#pragma clang diagnostic push +// We are silencing here because we don't know the selector beforehand +#pragma clang diagnostic ignored "-Warc-performSelector-leaks" + + id cls = NSClassFromString(@"SPDocument"); + if (cls == nullptr) { + return Status::failure("Could not load SPDocument class"); + } + + SEL sel = @selector(new); + if (![cls respondsToSelector:sel]) { + return Status::failure("SPDocument does not respond to new selector"); + } + + id document = [cls performSelector:sel]; + if (document == nullptr) { + return Status::failure("[SPDocument new] returned null"); + } + + auto cleanup_document = + scope_guard::create([&]() { CFRelease((__bridge CFTypeRef)document); }); + +#pragma clang diagnostic pop + + result = [document + reportForDataType:[NSString stringWithUTF8String:datatype.c_str()]]; + + return Status::success(); +} + +} // namespace osquery diff --git a/osquery/utils/tests/darwin/system_profiler.mm b/osquery/utils/tests/darwin/system_profiler.mm new file mode 100644 index 00000000000..d5a2e75f99d --- /dev/null +++ b/osquery/utils/tests/darwin/system_profiler.mm @@ -0,0 +1,25 @@ +/** + * Copyright (c) 2014-present, The osquery authors + * + * This source code is licensed as defined by the LICENSE file found in the + * root directory of this source tree. + * + * SPDX-License-Identifier: (Apache-2.0 OR GPL-2.0-only) + */ + +#include + +#include + +namespace osquery { + +class DarwinSystemProfilerTests : public testing::Test {}; + +TEST_F(DarwinSystemProfilerTests, test_getSystemProfilerReport) { + NSDictionary* __autoreleasing result; + Status status = getSystemProfilerReport("SPEthernetDataType", result); + EXPECT_TRUE(status.ok()) + EXPECT_NE([result count], 0U); +} + +} // namespace osquery \ No newline at end of file