From 0f5a35061ec0c58b33bea82e80d43a8a16bd0f53 Mon Sep 17 00:00:00 2001 From: Roberto Dip Date: Tue, 14 Feb 2023 10:23:19 -0300 Subject: [PATCH] don't filter DEP hosts by OS before ingesting and improve logs (#9815) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Related to https://github.com/fleetdm/fleet/issues/9653 I couldn't find any documentation to back this up, but I have a strong suspicion that the `os` field in the device sync response might come empty in some scenarios (particularly, when a laptop is brand new, which is hard to reproduce 😅) My thoughts are: 1. For the recently purchased MacBooks, `IngestMDMAppleDevicesFromDEPSync` didn't create an entry in the database, BUT `nanodep.Assigner.ProcessDeviceResponse` correctly assigned a DEP profile (the devices were able to enroll). Both methods filter by `op_type` but only ours filters by `os`. 2. I think this is safe-ish to do, as you will normally assign a MDM server per device type in ABM ![image](https://user-images.githubusercontent.com/4419992/218732609-0936e3a9-cadf-4485-9aa4-af2c9398cff9.png) 3. I have added extra logs to try to prove this hypothesis next time a brand new device comes in, let's keep an eye on and re-evaluate this approach. --- server/datastore/mysql/apple_mdm.go | 20 +++++++++++++------- server/datastore/mysql/apple_mdm_test.go | 8 ++++---- server/mdm/apple/apple_mdm.go | 2 ++ 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/server/datastore/mysql/apple_mdm.go b/server/datastore/mysql/apple_mdm.go index 546a8aea97..9a131fccb9 100644 --- a/server/datastore/mysql/apple_mdm.go +++ b/server/datastore/mysql/apple_mdm.go @@ -380,10 +380,12 @@ func insertMDMAppleHostDB( func (ds *Datastore) IngestMDMAppleDevicesFromDEPSync(ctx context.Context, devices []godep.Device) (int64, error) { if len(devices) < 1 { + level.Debug(ds.logger).Log("msg", "ingesting devices from DEP received < 1 device, skipping", "len(devices)", len(devices)) return 0, nil } - filteredDevices := filterMDMAppleDevices(devices) + filteredDevices := filterMDMAppleDevices(devices, ds.logger) if len(filteredDevices) < 1 { + level.Debug(ds.logger).Log("msg", "ingesting devices from DEP filtered all devices, skipping", "len(devices)", len(devices)) return 0, nil } @@ -397,6 +399,12 @@ func (ds *Datastore) IngestMDMAppleDevicesFromDEPSync(ctx context.Context, devic team, err := ds.TeamByName(ctx, name) switch { case errors.Is(err, sql.ErrNoRows): + level.Debug(ds.logger).Log( + "msg", + "ingesting devices from DEP: unable to find default team assigned in config, the devices won't be assigned to a team", + "team_name", + name, + ) // If the team doesn't exist, we still ingest the device, but it won't // belong to any team. case err != nil: @@ -590,14 +598,9 @@ func (ds *Datastore) UpdateHostTablesOnMDMUnenroll(ctx context.Context, uuid str }) } -func filterMDMAppleDevices(devices []godep.Device) []godep.Device { +func filterMDMAppleDevices(devices []godep.Device, logger log.Logger) []godep.Device { var filtered []godep.Device for _, device := range devices { - // We currently only support macOS devices so we screen out iOS - // and tvOS. - if strings.ToLower(device.OS) != "osx" { - continue - } // We currently only listen for an op_type of "added", the // other op_types are ambiguous and it would be needless to // ingest the device every single time we get an update. @@ -606,8 +609,11 @@ func filterMDMAppleDevices(devices []godep.Device) []godep.Device { // API call, Empty op_type come from the first call to // FetchDevices without a cursor. strings.ToLower(device.OpType) == "" { + level.Debug(logger).Log("msg", "filterMDMAppleDevices: adding device", "serial", device.SerialNumber, "op_type", device.OpType, "os", device.OS) filtered = append(filtered, device) + continue } + level.Debug(logger).Log("msg", "filterMDMAppleDevices: skipping device", "serial", device.SerialNumber, "op_type", device.OpType, "os", device.OS) } return filtered } diff --git a/server/datastore/mysql/apple_mdm_test.go b/server/datastore/mysql/apple_mdm_test.go index a6c93ea2bf..be77fd9280 100644 --- a/server/datastore/mysql/apple_mdm_test.go +++ b/server/datastore/mysql/apple_mdm_test.go @@ -45,17 +45,17 @@ func TestIngestMDMAppleDevicesFromDEPSync(t *testing.T) { {SerialNumber: "abc", Model: "MacBook Pro", OS: "OSX", OpType: "added"}, // ingested; new serial, macOS, "added" op type {SerialNumber: "abc", Model: "MacBook Pro", OS: "OSX", OpType: "added"}, // not ingested; duplicate serial {SerialNumber: hosts[0].HardwareSerial, Model: "MacBook Pro", OS: "OSX", OpType: "added"}, // not ingested; existing serial - {SerialNumber: "ijk", Model: "IPad Pro", OS: "iOS", OpType: "added"}, // not ingested; iOS - {SerialNumber: "tuv", Model: "Apple TV", OS: "tvOS", OpType: "added"}, // not ingested; tvOS + {SerialNumber: "ijk", Model: "MacBook Pro", OS: "", OpType: "added"}, // ingested; empty OS + {SerialNumber: "tuv", Model: "MacBook Pro", OS: "OSX", OpType: "modified"}, // not ingested; op type "modified" {SerialNumber: "xyz", Model: "MacBook Pro", OS: "OSX", OpType: "updated"}, // not ingested; op type "updated" {SerialNumber: "xyz", Model: "MacBook Pro", OS: "OSX", OpType: "deleted"}, // not ingested; op type "deleted" {SerialNumber: "xyz", Model: "MacBook Pro", OS: "OSX", OpType: "added"}, // ingested; new serial, macOS, "added" op type } - wantSerials = append(wantSerials, "abc", "xyz") + wantSerials = append(wantSerials, "abc", "xyz", "ijk") n, err := ds.IngestMDMAppleDevicesFromDEPSync(ctx, depDevices) require.NoError(t, err) - require.Equal(t, int64(2), n) // 2 new hosts ("abc", "xyz") + require.Equal(t, int64(3), n) // 3 new hosts ("abc", "xyz", "ijk") hosts = listHostsCheckCount(t, ds, fleet.TeamFilter{User: test.UserAdmin}, fleet.HostListOptions{}, len(wantSerials)) gotSerials := []string{} diff --git a/server/mdm/apple/apple_mdm.go b/server/mdm/apple/apple_mdm.go index d27d411631..a765ab3a85 100644 --- a/server/mdm/apple/apple_mdm.go +++ b/server/mdm/apple/apple_mdm.go @@ -124,6 +124,8 @@ func NewDEPSyncer( sentry.CaptureException(err) case n > 0: level.Info(kitlog.With(logger)).Log("msg", fmt.Sprintf("added %d new mdm device(s) to pending hosts", n)) + case n == 0: + level.Info(kitlog.With(logger)).Log("msg", "no DEP hosts to add") } return assigner.ProcessDeviceResponse(ctx, resp)