Fix `wifi_status` to correctly gather `network_name` on MacOS 14+ #8530

sgress454 · 2025-01-13T19:12:15Z

Fixes #8220
From Fleet: fleetdm/fleet#22195

This PR updates the source of the network_name column value in the wifi_status table. This value was previously copied from the ssid property of the interface retrieved via CWWiFiClient, but this is no longer available in MacOS 14.x (Sonoma). Instead, we'll now retrieve it from the system profiler, which still returns the current WiFi network name on newer OS X versions.

I'm open to ideas on what a good test addition would be here. Mocking the system profiler and CWWiFiClient responses would make a pretty watered-down test IMO. I could pull the code that maps MAC addresses to network names into a separate method and unit test that if that sounds worthwhile.

I did test this on a Mac OS 13 laptop (network_name still works as expected), on a MacOS 14 laptop without the change (network_name is missing) and with a MacOS 14 laptop with the change (network_name is populated).

linux-foundation-easycla · 2025-01-13T19:12:22Z

The committers listed above are authorized under a signed CLA.

✅ login: sgress454 / name: Scott Gress (c84fc7d)

sgress454 · 2025-01-13T19:12:32Z

osquery/tables/networking/darwin/wifi_status.mm

-    std::string interfaceName = "en0";
+    // Get the list of Wifi interfaces on the system.


This looks to have been hanging around for a long time, but it's not used anywhere I can see.

sgress454 · 2025-01-13T19:12:48Z

osquery/tables/networking/darwin/wifi_status.mm

+    // Make a map of MAC addresses to network names.
+    NSMutableDictionary* wifiNetworks = [NSMutableDictionary dictionary];
+    for (NSDictionary* item in [result objectForKey:@"_items"]) {
+      // Get the item's airport intefaces, which will usually include at least
+      // a wifi card and an Apple Wireless Direct Link (AWDL) interface.
+      NSArray* airportInterfaces =
+          [item objectForKey:@"spairport_airport_interfaces"];
+      // Get the wifi interface (the one starting with "en").
+      NSDictionary* wifiInterface = [[airportInterfaces
+          filteredArrayUsingPredicate:
+              [NSPredicate predicateWithFormat:@"_name BEGINSWITH %@", @"en"]]
+          lastObject];
+      // Add the network name to the map, indexed by hardware address.
+      wifiNetworks[
+          [wifiInterface objectForKey:@"spairport_wireless_mac_address"]] =
+          [[wifiInterface objectForKey:@"spairport_current_network_information"]
+              objectForKey:@"_name"];
+    }


Support systems w/ multiple network cards so that each row of the table shows the correct network.

directionless · 2025-01-15T03:05:32Z

osquery/tables/networking/darwin/wifi_status.mm

+    // Get Airport data from system profiler, in order to get the current
+    // network name for systems that don't return it via CWWiFiClient.
+    NSDictionary* __autoreleasing result;
+    Status status = getSystemProfilerReport("SPAirPortDataType", result);


How's the performance here? Right now. this table takes 0.5s - 1s on my machine to run. In contrast, executing system_profiler SPAirPortDataType takes 4s

Thanks for mentioning this. I'll be sure to call out performance implications in future PRs.

This change does indeed add about 4 seconds to the query in my tests as well. A few optimizations come to mind:

I could first check whether the wifi_network field (or *) is being requested, and only do the system profiler call if needed.

I could further check the MacOS version and only run this extra step for 14+, although that will have diminishing returns.

The networksetup utility can provide it, but it's against the guidelines to shell out to external processes. I could maybe get it from the Apple80211 framework that networksetup uses, in a similar way to how we use the private SystemProfiler framework. I can't speak to how brittle that is compared to what we're already doing with the private SPSupport.framework, or how much of a rabbit hole getting it to work would be, or if the community would accept it. But if it worked it'd probably be faster -- networksetup is very fast compared to system_profiler.

Maybe it's all wall time? We should look into the user and system time too.

time system_profiler -json SPAirPortDataType [...] system_profiler -json SPAirPortDataType 0.15s user 0.38s system 13% cpu 3.897 total

If user and system time are a-ok maybe this is a-ok?

Perhaps! It's a pretty big delta though, and anecdotally experienced on two different machines, so my guess would be that the command is waiting for some blocking I/O calls. It could be faster on different systems but if this is just the lag it takes to scan Airport devices, it'll probably affect most Macs. Profiling system_profiler is another rabbit hole I could fall down 🐇

There's also the question of how important this is -- it's clearly a lot slower, but is it now too slow? What's the expectation here?

I could first check whether the wifi_network field (or *) is being requested, and only do the system profiler call if needed.

I'm not sure this is technically possible, though if it was I think it would be worth thinking about.

I could maybe get it from the Apple80211 framework that networksetup uses

Seems good to find out. I agree it might be a rabbit hole, and I don't want to sign you up for it. But maybe timebox something and then we can make an informed decision?

I don't think the delay should be a dealbreaker personally, unless it scales with the number of table rows or something-- but I bet it's just the startup time of loading system-profiler.

The networksetup utility can provide it, but it's against the guidelines to shell out to external processes. I could maybe get it from the Apple80211 framework that networksetup uses

The last time I looked into that I believe it was locked behind the location service entitlement, which looked like a mess.

My suspicion would be along the lines of what @lucasmrod suggested -- it's probably blocking on I/O most of the time. Seems like that performance should be okay even if we wish the table were quicker?

Updated this to only do the extra system profiler call if "network_name" is one of the columns requested (this works if SELECT * is used as well. The Apple80211 path seems fraught but this way we at least silo the performance hit.

lucasmrod

LGTM.

I'm assuming user and system time are low with the system profiler approach (and high wall_time is not an issue).

directionless · 2025-01-22T20:37:33Z

I'm assuming user and system time are low with the system profiler approach (and high wall_time is not an issue).

While I am still a bit agnostic, I think we should be very cautious about that attitude. Yes, user and system time impact end user perception about whether osquery is performant. But wall_time matters as well. Osquery is (mostly) single threaded, so if this query is running, it will block all other distributed queries until completion.

sgress454 · 2025-01-23T21:51:49Z

osquery/tables/networking/darwin/wifi_status.mm

+    // Make a map of interface names to network names.
+    NSMutableDictionary* wifiNetworks = [NSMutableDictionary dictionary];
+    for (NSDictionary* item in [result objectForKey:@"_items"]) {
+      // Get the item's airport intefaces, which will usually include at least
+      // a wifi card and an Apple Wireless Direct Link (AWDL) interface.
+      NSArray* airportInterfaces =
+          [item objectForKey:@"spairport_airport_interfaces"];
+      // Get the wifi interface (the one starting with "en").
+      NSDictionary* wifiInterface = [[airportInterfaces
+          filteredArrayUsingPredicate:
+              [NSPredicate predicateWithFormat:@"_name BEGINSWITH %@", @"en"]]
+          lastObject];
+      // Add the network name to the map, indexed by interface name.
+      wifiNetworks[[wifiInterface objectForKey:@"_name"]] =
+          [[wifiInterface objectForKey:@"spairport_current_network_information"]
+              objectForKey:@"_name"];
+    }


Re-worked this to build the map based on interface name (e.g. en0) instead of MAC address. When testing this on a newer Macbook, it had the "Private WiFi Address" feature turned on which made the address in the system profiler not match the address from CWWiFiClient, which in turn caused a fault when trying to read from the map below.

sgress454 · 2025-01-23T21:52:39Z

osquery/tables/networking/darwin/wifi_status.mm

+      r["network_name"] =
+          std::string([[wifiNetworks objectForKey:[interface interfaceName]]
+                           ?: @"" UTF8String]);


Get the network name from the map we created, defaulting to "" if for some reason we don't have a map entry for it.

directionless · 2025-01-24T03:47:14Z

osquery/tables/networking/darwin/wifi_status.mm

+
+    // To get the network name we need to use the system profiler.
+    // Since this is a performance hit, only do it if we need to.
+    if (context.isColumnUsed("network_name")) {


Whoa. I didn't realize we could do that. Cool!

lucasmrod · 2025-01-24T11:38:07Z

osquery/tables/networking/darwin/wifi_status.mm

+      if (!status.ok()) {
+        LOG(ERROR) << "failed to get SPAirPortDataType config: "
+                   << status.getMessage();
+        return {};


If system profiler fails we should return empty for network_name (which is currently the case) instead of no rows. (Logging the error is a good idea though.)

good call, updated.

sgress454 · 2025-01-29T16:25:48Z

@directionless @lucasmrod checks are green on this, if I can get another +1 🙏

sgress454 requested review from a team as code owners January 13, 2025 19:12

sgress454 commented Jan 13, 2025

View reviewed changes

directionless reviewed Jan 15, 2025

View reviewed changes

sgress454 force-pushed the sgress454/22195-fix-wifi-network-in-wifi-status-table branch from 699206f to 96c27ab Compare January 15, 2025 18:23

lucasmrod approved these changes Jan 22, 2025

View reviewed changes

sgress454 force-pushed the sgress454/22195-fix-wifi-network-in-wifi-status-table branch from 96c27ab to 32e1e12 Compare January 23, 2025 21:48

sgress454 commented Jan 23, 2025

View reviewed changes

sgress454 force-pushed the sgress454/22195-fix-wifi-network-in-wifi-status-table branch from 32e1e12 to 9895c76 Compare January 23, 2025 22:04

directionless previously approved these changes Jan 24, 2025

View reviewed changes

lucasmrod reviewed Jan 24, 2025

View reviewed changes

get wifi network from system profiler

c84fc7d

sgress454 dismissed directionless’s stale review via c84fc7d January 27, 2025 17:21

sgress454 force-pushed the sgress454/22195-fix-wifi-network-in-wifi-status-table branch from 9895c76 to c84fc7d Compare January 27, 2025 17:21

directionless approved these changes Jan 29, 2025

View reviewed changes

directionless added bug virtual tables macOS labels Jan 29, 2025

directionless changed the title ~~Fix network_name in wifi status table for MacOS 14+~~ Fix wifi_status to correctly gather network_name on MacOS 14+ Jan 29, 2025

directionless merged commit 745cdba into osquery:master Jan 29, 2025
22 checks passed

		std::string interfaceName = "en0";
		// Get the list of Wifi interfaces on the system.

Uh oh!

Fix wifi_status to correctly gather network_name on MacOS 14+ #8530

Fix wifi_status to correctly gather network_name on MacOS 14+ #8530

Uh oh!

Conversation

sgress454 commented Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linux-foundation-easycla bot commented Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sgress454 Jan 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lucasmrod Jan 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lucasmrod left a comment

Choose a reason for hiding this comment

Uh oh!

directionless commented Jan 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lucasmrod Jan 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sgress454 commented Jan 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Fix `wifi_status` to correctly gather `network_name` on MacOS 14+ #8530

Fix `wifi_status` to correctly gather `network_name` on MacOS 14+ #8530

sgress454 commented Jan 13, 2025 •

edited

Loading

linux-foundation-easycla bot commented Jan 13, 2025 •

edited

Loading

sgress454 Jan 16, 2025 •

edited

Loading

lucasmrod Jan 16, 2025 •

edited

Loading

lucasmrod Jan 24, 2025 •

edited

Loading