Refactor macOS system profile report retrieval (osquery#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.
Smjert · Feb 9, 2024 · 2bd7e86 · 2bd7e86
1 parent 957960a
commit 2bd7e86
Show file tree

Hide file tree

Showing 9 changed files with 204 additions and 143 deletions.
diff --git 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
@@ -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
@@ -12,11 +12,7 @@
 
 #include <osquery/core/tables.h>
 #include <osquery/logger/logger.h>
-
-@interface SPDocument : NSDocument {
-}
-- (id)reportForDataType:(id)arg1;
-@end
+#include <osquery/utils/darwin/system_profiler.h>
 
 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<void()> 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
@@ -12,20 +12,13 @@
 #include <osquery/core/core.h>
 #include <osquery/core/tables.h>
 #include <osquery/logger/logger.h>
-#include <osquery/utils/scope_guard.h>
+#include <osquery/utils/darwin/system_profiler.h>
 
-#import <AppKit/NSDocument.h>
 #include <CoreFoundation/CoreFoundation.h>
-#import <Foundation/Foundation.h>
 #include <IOKit/IOKitKeys.h>
 #include <IOKit/IOKitLib.h>
 #include <mach/mach_error.h>
 
-@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
@@ -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
@@ -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
@@ -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 <osquery/utils/status/status.h>
+
+#import <Foundation/Foundation.h>
+
+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