darwin: Improve disk_events add detection (#3332)

osquery/events/darwin/diskarbitration.cpp

@@ -22,10 +22,11 @@
 #include "osquery/events/darwin/diskarbitration.h"
 
 namespace fs = boost::filesystem;
+namespace errc = boost::system::errc;
 
 namespace osquery {
 
-const std::string kIOHIDXClassPath = "IOService:/IOResources/IOHDIXController/";
+const std::string kIOHIDXClassPath{"IOService:/IOResources/IOHDIXController/"};
 
 REGISTER(DiskArbitrationEventPublisher, "event_publisher", "diskarbitration");
 
@@ -83,7 +84,7 @@ void DiskArbitrationEventPublisher::tearDown() {
 }
 
 void DiskArbitrationEventPublisher::DiskAppearedCallback(DADiskRef disk,
-                                                         void *context) {
+                                                         void* context) {
   auto ec = createEventContext();
 
   CFDictionaryRef disk_properties = DADiskCopyDescription(disk);
@@ -120,38 +121,43 @@ void DiskArbitrationEventPublisher::DiskAppearedCallback(DADiskRef disk,
               disk_properties, kDADiskDescriptionMediaWholeKey))) {
         ec->checksum = extractUdifChecksum(ec->path);
       }
+    } else {
+      // There was no interface location.
+      ec->path = getProperty(kDADiskDescriptionDevicePathKey, disk_properties);
     }
     CFRelease(protocol_properties);
   } else {
     ec->path = "";
   }
 
-  fire("add", ec, disk_properties);
+  if (ec->path.find("/SSD0@0") == std::string::npos) {
+    // This is not an internal SSD.
+    fire("add", ec, disk_properties);
+  }
 
   CFRelease(disk_properties);
   IOObjectRelease(entry);
 }
 
 void DiskArbitrationEventPublisher::DiskDisappearedCallback(DADiskRef disk,
-                                                            void *context) {
-
+                                                            void* context) {
   CFDictionaryRef disk_properties = DADiskCopyDescription(disk);
   fire("remove", createEventContext(), disk_properties);
   CFRelease(disk_properties);
 }
 
 bool DiskArbitrationEventPublisher::shouldFire(
-    const DiskArbitrationSubscriptionContextRef &sc,
-    const DiskArbitrationEventContextRef &ec) const {
-
+    const DiskArbitrationSubscriptionContextRef& sc,
+    const DiskArbitrationEventContextRef& ec) const {
   // We want events for physical disks as well
   if (sc->physical_disks) {
     return true;
   } else {
-    // We want only virtual disk (DMG) events
+    // We 'could' only want only virtual disk (DMG) events
     if (ec->action == "add") {
       // Filter events by matching on Virtual Interface based on IO device path
-      return (boost::starts_with(ec->device_path, kIOHIDXClassPath));
+      // return (boost::starts_with(ec->device_path, kIOHIDXClassPath));
+      return true;
     } else {
       return true;
     }
@@ -159,14 +165,17 @@ bool DiskArbitrationEventPublisher::shouldFire(
 }
 
 std::string DiskArbitrationEventPublisher::extractUdifChecksum(
-    const std::string &path_str) {
+    const std::string& path_str) {
   fs::path path = path_str;
   if (!pathExists(path).ok() || !isReadable(path).ok()) {
     return "";
   }
-  if (!fs::is_regular_file(path)) {
+
+  boost::system::error_code ec;
+  if (!fs::is_regular_file(path, ec) || ec.value() != errc::success) {
     return "";
   }
+
   // The koly trailer (footer) is 512 bytes
   // http://newosxbook.com/DMG.html
   if (fs::file_size(path) < 512) {
@@ -177,7 +186,7 @@ std::string DiskArbitrationEventPublisher::extractUdifChecksum(
   if (dmg_file.is_open()) {
     dmg_file.seekg(-512L, std::ios::end);
 
-    char *buffer = new char[4];
+    char* buffer = new char[4];
     dmg_file.read(buffer, 4);
     std::string koly_signature;
     koly_signature.assign(buffer, 4);
@@ -191,13 +200,13 @@ std::string DiskArbitrationEventPublisher::extractUdifChecksum(
 
     uint32_t checksum_size;
     dmg_file.seekg(-156L, std::ios::end);
-    dmg_file.read((char *)&checksum_size, sizeof(checksum_size));
+    dmg_file.read((char*)&checksum_size, sizeof(checksum_size));
     // checksum_size is in big endian and we need to byte swap
     checksum_size = CFSwapInt32(checksum_size);
 
     dmg_file.seekg(-152L, std::ios::end); // checksum offset
-    unsigned char *u_buffer = new unsigned char[checksum_size];
-    dmg_file.read((char *)u_buffer, checksum_size);
+    unsigned char* u_buffer = new unsigned char[checksum_size];
+    dmg_file.read((char*)u_buffer, checksum_size);
     // we don't want to byte swap checksum as disk utility/hdiutil doesn't
     std::stringstream checksum;
     for (size_t i = 0; i < checksum_size; i++) {
@@ -214,16 +223,15 @@ std::string DiskArbitrationEventPublisher::extractUdifChecksum(
 }
 
 void DiskArbitrationEventPublisher::fire(
-    const std::string &action,
-    const DiskArbitrationEventContextRef &ec,
-    const CFDictionaryRef &dict) {
-
+    const std::string& action,
+    const DiskArbitrationEventContextRef& ec,
+    const CFDictionaryRef& dict) {
   ec->action = action;
   ec->name = getProperty(kDADiskDescriptionMediaNameKey, dict);
-  ec->bsd_name = getProperty(kDADiskDescriptionMediaBSDNameKey, dict);
+  ec->device = "/dev/" + getProperty(kDADiskDescriptionMediaBSDNameKey, dict);
   ec->uuid = getProperty(kDADiskDescriptionVolumeUUIDKey, dict);
   ec->size = getProperty(kDADiskDescriptionMediaSizeKey, dict);
-  ec->ejectable = getProperty(kDADiskDescriptionMediaEjectableKey, dict);
+  ec->ejectable = getProperty(kDADiskDescriptionMediaRemovableKey, dict);
   ec->mountable = getProperty(kDADiskDescriptionVolumeMountableKey, dict);
   ec->writable = getProperty(kDADiskDescriptionMediaWritableKey, dict);
   ec->content = getProperty(kDADiskDescriptionMediaContentKey, dict);
@@ -231,12 +239,15 @@ void DiskArbitrationEventPublisher::fire(
   ec->vendor = getProperty(kDADiskDescriptionDeviceVendorKey, dict);
   ec->filesystem = getProperty(kDADiskDescriptionVolumeKindKey, dict);
   ec->disk_appearance_time = getProperty(CFSTR(kDAAppearanceTime_), dict);
+  if (ec->path.find("IOService:/") == 0) {
+    ec->path = ec->device;
+  }
 
   EventFactory::fire<DiskArbitrationEventPublisher>(ec);
 }
 
 std::string DiskArbitrationEventPublisher::getProperty(
-    const CFStringRef &property, const CFDictionaryRef &dict) {
+    const CFStringRef& property, const CFDictionaryRef& dict) {
   CFTypeRef value = (CFTypeRef)CFDictionaryGetValue(dict, property);
   if (value == nullptr) {
     return "";

osquery/events/darwin/diskarbitration.h

@@ -34,7 +34,7 @@ struct DiskArbitrationEventContext : public EventContext {
   std::string path;
   std::string device_path;
   std::string name;
-  std::string bsd_name;
+  std::string device;
   std::string uuid;
   std::string size;
   std::string ejectable;
@@ -63,28 +63,28 @@ class DiskArbitrationEventPublisher
 
   void tearDown() override;
 
-  bool shouldFire(const DiskArbitrationSubscriptionContextRef &sc,
-                  const DiskArbitrationEventContextRef &ec) const override;
+  bool shouldFire(const DiskArbitrationSubscriptionContextRef& sc,
+                  const DiskArbitrationEventContextRef& ec) const override;
 
   Status run() override;
 
-  static void DiskAppearedCallback(DADiskRef disk, void *context);
+  static void DiskAppearedCallback(DADiskRef disk, void* context);
 
-  static void DiskDisappearedCallback(DADiskRef disk, void *context);
+  static void DiskDisappearedCallback(DADiskRef disk, void* context);
 
  private:
   void restart();
 
   void stop() override;
 
-  static std::string getProperty(const CFStringRef &property,
-                                 const CFDictionaryRef &dict);
+  static std::string getProperty(const CFStringRef& property,
+                                 const CFDictionaryRef& dict);
 
-  static std::string extractUdifChecksum(const std::string &path);
+  static std::string extractUdifChecksum(const std::string& path);
 
-  static void fire(const std::string &action,
-                   const DiskArbitrationEventContextRef &ec,
-                   const CFDictionaryRef &dict);
+  static void fire(const std::string& action,
+                   const DiskArbitrationEventContextRef& ec,
+                   const CFDictionaryRef& dict);
 
  private:
   /// Disk arbitration session.

osquery/tables/events/darwin/disk_events.cpp

@@ -41,7 +41,7 @@ Status DiskEventSubscriber::Callback(const ECRef& ec, const SCRef& sc) {
   r["action"] = ec->action;
   r["path"] = ec->path;
   r["name"] = ec->name;
-  r["bsd_name"] = "/dev/" + ec->bsd_name;
+  r["device"] = ec->device;
   r["uuid"] = ec->uuid;
   r["size"] = ec->size;
   r["ejectable"] = ec->ejectable;

specs/darwin/disk_events.table

@@ -4,7 +4,8 @@ schema([
     Column("action", TEXT, "Appear or disappear"),
     Column("path", TEXT, "Path of the DMG file accessed"),
     Column("name", TEXT, "Disk event name"),
-    Column("bsd_name", TEXT, "Disk event BSD name"),
+    Column("device", TEXT, "Disk event BSD name",
+        aliases=["bsd_name"]),
     Column("uuid", TEXT, "UUID of the volume inside DMG if available"),
     Column("size", BIGINT, "Size of partition in bytes"),
     Column("ejectable", INTEGER, "1 if ejectable, 0 if not"),