From 7fdaa92f57bccb6a21b1692b597a284840fc0108 Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky <2685025+getvictor@users.noreply.github.com> Date: Wed, 27 May 2026 17:02:45 +0000 Subject: [PATCH 1/7] windows_mdm: link enrollment row via DevDetail at first management session Closes the race after Windows BYOD MDM enrollment (Settings > Access work or school > Connect) where mdm_windows_enrollments.host_uuid stayed empty for ~10s while osquery's distributed-read cycle ran directIngestMDMDeviceID Windows. During that gap any server-side lookup keyed on host UUID via MDMWindowsGetEnrolledDeviceWithHostUUID returned NotFound. processIncomingMDMCmds now inspects unlinked enrollments on every management session: it parses any incoming Results for ./DevDetail/Ext/Microsoft/SMBIOSSerialNumber, looks up the Windows host by hardware_serial, and updates host_uuid. If still unlinked after processing the incoming message, it appends a Get for that LocURI to the response so the device replies on the next round-trip. The Get is idempotent and reinjected each session until linkage succeeds. The post-link UPN/SCIM/DEP bookkeeping previously inlined in directIngestMDMDeviceIDWindows is extracted into a shared helper (osquery_utils.LinkWindowsHostMDMEnrollment) so both the new SyncML path and the osquery direct-ingest backstop run it exactly once per linkage. New datastore method WindowsHostLiteByHardwareSerial does a Windows-only serial lookup and returns NotFound when two Windows hosts share a serial, so we never mis-link on virtualization-shared SMBIOS values. For Autopilot and Entra-during-OOBE the host record does not exist until fleetd installs later in ESP, so the osquery backstop and the name-based fallback in setup_experience.go remain in place for those flows. Refs #45380 --- .../45380-windows-mdm-enrollment-row-linkage | 1 + server/datastore/mysql/microsoft_mdm.go | 37 ++++ server/datastore/mysql/microsoft_mdm_test.go | 46 +++++ server/fleet/datastore.go | 6 + server/mock/datastore_mock.go | 12 ++ server/service/mdm_test.go | 159 ++++++++++++++++++ server/service/microsoft_mdm.go | 96 ++++++++++- server/service/osquery_utils/queries.go | 106 ++++++------ 8 files changed, 408 insertions(+), 55 deletions(-) create mode 100644 changes/45380-windows-mdm-enrollment-row-linkage diff --git a/changes/45380-windows-mdm-enrollment-row-linkage b/changes/45380-windows-mdm-enrollment-row-linkage new file mode 100644 index 00000000000..c320054aa51 --- /dev/null +++ b/changes/45380-windows-mdm-enrollment-row-linkage @@ -0,0 +1 @@ +- Fixed a race after Windows BYOD MDM enrollment (Settings > Access work or school > Connect) where `mdm_windows_enrollments.host_uuid` stayed empty for several seconds, causing server-side enrollment lookups to miss. The enrollment is now linked to the Fleet host record at the first management session via OMA-DM DevDetail/SMBIOSSerialNumber instead of waiting for osquery's distributed-read backfill. diff --git a/server/datastore/mysql/microsoft_mdm.go b/server/datastore/mysql/microsoft_mdm.go index 36fa627bced..69566a43b80 100644 --- a/server/datastore/mysql/microsoft_mdm.go +++ b/server/datastore/mysql/microsoft_mdm.go @@ -176,6 +176,43 @@ func (ds *Datastore) MDMWindowsGetUnlinkedEnrolledDeviceWithDeviceName(ctx conte return &winMDMDevice, nil } +// WindowsHostLiteByHardwareSerial looks up a Windows host by its hardware_serial. It is used to link an unlinked Windows +// MDM enrollment (mdm_windows_enrollments.host_uuid = ”) to its host using DevDetail/Ext/Microsoft/SMBIOSSerialNumber +// reported during the first SyncML management session. The platform filter avoids cross-platform collisions on shared +// serials. If multiple Windows hosts share the same serial (e.g. VM gold images that did not regenerate SMBIOS), the +// caller cannot pick safely so we return NotFound and let the osquery direct-ingest backstop resolve linkage later. +func (ds *Datastore) WindowsHostLiteByHardwareSerial(ctx context.Context, hardwareSerial string) (*fleet.HostLite, error) { + if hardwareSerial == "" { + return nil, ctxerr.Wrap(ctx, notFound("Host").WithMessage("empty hardware serial")) + } + const stmt = ` + SELECT + h.id, + h.team_id, + h.osquery_host_id, + COALESCE(h.node_key, '') AS node_key, + h.computer_name, + h.hostname, + h.uuid, + h.hardware_model, + h.hardware_serial, + h.distributed_interval, + h.config_tls_refresh, + COALESCE(hst.seen_time, h.created_at) AS seen_time + FROM hosts h + LEFT JOIN host_seen_times hst ON h.id = hst.host_id + WHERE h.hardware_serial = ? AND h.platform = 'windows' + LIMIT 2` + var hosts []*fleet.HostLite + if err := sqlx.SelectContext(ctx, ds.reader(ctx), &hosts, stmt, hardwareSerial); err != nil { + return nil, ctxerr.Wrap(ctx, err, "select windows host by hardware serial") + } + if len(hosts) != 1 { + return nil, ctxerr.Wrap(ctx, notFound("Host").WithMessage(hardwareSerial)) + } + return hosts[0], nil +} + // HasWindowsSetupExperienceItemsForTeam returns true if any active Windows setup-experience software // installers with install_during_setup=TRUE are configured for the given team. teamID=0 means "no team / // global", matching the value EnqueueSetupExperienceItems passes in for hosts on no team. diff --git a/server/datastore/mysql/microsoft_mdm_test.go b/server/datastore/mysql/microsoft_mdm_test.go index d9053ea2013..ff9dffc88fd 100644 --- a/server/datastore/mysql/microsoft_mdm_test.go +++ b/server/datastore/mysql/microsoft_mdm_test.go @@ -70,6 +70,7 @@ func TestMDMWindows(t *testing.T) { {"TestMDMWindowsInsertCommandSkipsUnenrolledHosts", testMDMWindowsInsertCommandSkipsUnenrolledHosts}, {"TestCleanupWindowsMDMCommandQueue", testCleanupWindowsMDMCommandQueue}, {"TestMDMWindowsGetUnlinkedEnrolledDeviceWithDeviceName", testMDMWindowsGetUnlinkedEnrolledDeviceWithDeviceName}, + {"TestWindowsHostLiteByHardwareSerial", testWindowsHostLiteByHardwareSerial}, } for _, c := range cases { @@ -6351,3 +6352,48 @@ func testMDMWindowsGetUnlinkedEnrolledDeviceWithDeviceName(t *testing.T, ds *Dat require.True(t, fleet.IsNotFound(err), "all matching rows now linked; method must return NotFound") } + +func testWindowsHostLiteByHardwareSerial(t *testing.T, ds *Datastore) { + ctx := t.Context() + + // 1. Empty serial → NotFound. + _, err := ds.WindowsHostLiteByHardwareSerial(ctx, "") + require.Error(t, err) + require.True(t, fleet.IsNotFound(err)) + + // 2. No matching host → NotFound. + _, err = ds.WindowsHostLiteByHardwareSerial(ctx, "SERIAL-DOES-NOT-EXIST") + require.Error(t, err) + require.True(t, fleet.IsNotFound(err)) + + // 3. Single matching Windows host → returns it. + winHost := test.NewHost(t, ds, "win-by-serial", "10.0.0.50", "win-by-serial-key", "win-by-serial-uuid", time.Now()) + winHost.Platform = "windows" + winHost.HardwareSerial = "SERIAL-WIN-1" + require.NoError(t, ds.UpdateHost(ctx, winHost)) + + got, err := ds.WindowsHostLiteByHardwareSerial(ctx, "SERIAL-WIN-1") + require.NoError(t, err) + require.Equal(t, winHost.ID, got.ID) + require.Equal(t, winHost.UUID, got.UUID) + + // 4. Non-Windows host with the same serial must not match (platform filter prevents cross-platform mis-linkage). + macHost := test.NewHost(t, ds, "mac-same-serial", "10.0.0.51", "mac-same-serial-key", "mac-same-serial-uuid", time.Now()) + macHost.Platform = "darwin" + macHost.HardwareSerial = "SERIAL-WIN-1" + require.NoError(t, ds.UpdateHost(ctx, macHost)) + + got, err = ds.WindowsHostLiteByHardwareSerial(ctx, "SERIAL-WIN-1") + require.NoError(t, err) + require.Equal(t, winHost.ID, got.ID, "must return the Windows host even when a darwin host shares the serial") + + // 5. Two Windows hosts sharing the same serial → NotFound (linkage would be ambiguous). + winHostDup := test.NewHost(t, ds, "win-by-serial-dup", "10.0.0.52", "win-by-serial-dup-key", "win-by-serial-dup-uuid", time.Now()) + winHostDup.Platform = "windows" + winHostDup.HardwareSerial = "SERIAL-WIN-1" + require.NoError(t, ds.UpdateHost(ctx, winHostDup)) + + _, err = ds.WindowsHostLiteByHardwareSerial(ctx, "SERIAL-WIN-1") + require.Error(t, err) + require.True(t, fleet.IsNotFound(err), "two Windows hosts share the serial; method must refuse to pick") +} diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index eaa19fb95d8..9740bb67c6f 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -2111,6 +2111,12 @@ type Datastore interface { // find a row because the host->enrollment link has not been resolved yet. MDMWindowsGetUnlinkedEnrolledDeviceWithDeviceName(ctx context.Context, deviceName string) (*MDMWindowsEnrolledDevice, error) + // WindowsHostLiteByHardwareSerial returns a HostLite for the Windows host whose hardware_serial matches the given + // serial. Used to link an unlinked Windows MDM enrollment to a host using SMBIOS data reported via OMA-DM DevDetail + // before osquery's directIngestMDMDeviceIDWindows has run. Returns NotFound if no Windows host matches or if multiple + // Windows hosts share the same serial (which would make the linkage ambiguous). + WindowsHostLiteByHardwareSerial(ctx context.Context, hardwareSerial string) (*HostLite, error) + // MDMWindowsDeleteEnrolledDeviceWithDeviceID deletes a give MDMWindowsEnrolledDevice entry from the database using the device id MDMWindowsDeleteEnrolledDeviceWithDeviceID(ctx context.Context, mdmDeviceID string) error diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index baf38821023..327a4314bdd 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -1317,6 +1317,8 @@ type MDMWindowsGetEnrolledDeviceWithHostUUIDFunc func(ctx context.Context, hostU type MDMWindowsGetUnlinkedEnrolledDeviceWithDeviceNameFunc func(ctx context.Context, deviceName string) (*fleet.MDMWindowsEnrolledDevice, error) +type WindowsHostLiteByHardwareSerialFunc func(ctx context.Context, hardwareSerial string) (*fleet.HostLite, error) + type MDMWindowsDeleteEnrolledDeviceWithDeviceIDFunc func(ctx context.Context, mdmDeviceID string) error type MDMWindowsInsertCommandForHostsFunc func(ctx context.Context, hostUUIDs []string, cmd *fleet.MDMWindowsCommand) error @@ -3947,6 +3949,9 @@ type DataStore struct { MDMWindowsGetUnlinkedEnrolledDeviceWithDeviceNameFunc MDMWindowsGetUnlinkedEnrolledDeviceWithDeviceNameFunc MDMWindowsGetUnlinkedEnrolledDeviceWithDeviceNameFuncInvoked bool + WindowsHostLiteByHardwareSerialFunc WindowsHostLiteByHardwareSerialFunc + WindowsHostLiteByHardwareSerialFuncInvoked bool + MDMWindowsDeleteEnrolledDeviceWithDeviceIDFunc MDMWindowsDeleteEnrolledDeviceWithDeviceIDFunc MDMWindowsDeleteEnrolledDeviceWithDeviceIDFuncInvoked bool @@ -9511,6 +9516,13 @@ func (s *DataStore) MDMWindowsGetUnlinkedEnrolledDeviceWithDeviceName(ctx contex return s.MDMWindowsGetUnlinkedEnrolledDeviceWithDeviceNameFunc(ctx, deviceName) } +func (s *DataStore) WindowsHostLiteByHardwareSerial(ctx context.Context, hardwareSerial string) (*fleet.HostLite, error) { + s.mu.Lock() + s.WindowsHostLiteByHardwareSerialFuncInvoked = true + s.mu.Unlock() + return s.WindowsHostLiteByHardwareSerialFunc(ctx, hardwareSerial) +} + func (s *DataStore) MDMWindowsDeleteEnrolledDeviceWithDeviceID(ctx context.Context, mdmDeviceID string) error { s.mu.Lock() s.MDMWindowsDeleteEnrolledDeviceWithDeviceIDFuncInvoked = true diff --git a/server/service/mdm_test.go b/server/service/mdm_test.go index ed2876dfe26..a356b08df25 100644 --- a/server/service/mdm_test.go +++ b/server/service/mdm_test.go @@ -3152,3 +3152,162 @@ func TestGetDeviceSoftwareMDMCommandResultsVPPMetadata(t *testing.T) { require.False(t, ds.GetVPPAppInstallStatusByCommandUUIDFuncInvoked) }) } + +// TestProcessIncomingMDMCmdsDevDetailLinkage exercises the OMA-DM DevDetail-based linkage path that closes the race +// where mdm_windows_enrollments.host_uuid is empty after Azure / automatic enrollment. See issue #45380. The primary +// linkage mechanism is implemented in processIncomingMDMCmds: when an enrollment row has empty host_uuid, the server +// (a) parses any incoming Results for ./DevDetail/Ext/Microsoft/SMBIOSSerialNumber and links the host, then +// (b) injects a Get for the same URI into the outgoing response so the device replies on the next round-trip. +func TestProcessIncomingMDMCmdsDevDetailLinkage(t *testing.T) { + const ( + testDeviceID = "test-device-id" + testSerial = "ABC123XYZ" + testHostUUID = "host-uuid-from-osquery" + testHostID uint = 99 + ) + + // buildReqMsg builds a minimal valid SyncML request. extraBody (if non-empty) is inserted into ; use it to + // inject a with the device's reply to our DevDetail Get. + buildReqMsg := func(t *testing.T, extraBody string) *fleet.SyncML { + t.Helper() + raw := fmt.Sprintf(` + + 1.2 + DM/1.2 + 1 + 1 + %s + + + %s + + + `, testDeviceID, extraBody) + reqMsg := &fleet.SyncML{} + require.NoError(t, xml.Unmarshal([]byte(raw), reqMsg)) + reqMsg.Raw = []byte(raw) + return reqMsg + } + + // newSvc builds a service with the stubs that processIncomingMDMCmds always touches; per-subtest stubs override. + newSvc := func(t *testing.T) (*Service, *mock.Store, context.Context) { + t.Helper() + ds := new(mock.Store) + opts := &TestServerOpts{} + svc, ctx := newTestService(t, ds, nil, nil, opts) + var svcImpl *Service + switch v := svc.(type) { + case validationMiddleware: + svcImpl = v.Service.(*Service) + case *Service: + svcImpl = v + } + ds.MDMWindowsSaveResponseFunc = func(ctx context.Context, _ *fleet.MDMWindowsEnrolledDevice, _ fleet.EnrichedSyncML, _ []string) (*fleet.MDMWindowsSaveResponseResult, error) { + return nil, nil + } + ds.GetWindowsMDMCommandsForResendingFunc = func(ctx context.Context, deviceID string, cmdUUIDs []string) ([]*fleet.MDMWindowsCommand, error) { + return nil, nil + } + return svcImpl, ds, ctx + } + + // hasGetForDevDetailSerial reports whether responseCmds contains a Get for the SMBIOS serial number URI. + hasGetForDevDetailSerial := func(cmds []*fleet.SyncMLCmd) bool { + for _, c := range cmds { + if c.XMLName.Local != fleet.CmdGet || len(c.Items) == 0 || c.Items[0].Target == nil { + continue + } + if *c.Items[0].Target == devDetailSMBIOSSerialNumberURI { + return true + } + } + return false + } + + t.Run("unlinked enrollment: Get for DevDetail SMBIOSSerialNumber is injected", func(t *testing.T) { + svc, ds, ctx := newSvc(t) + enrolledDevice := &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, HostUUID: ""} + reqMsg := buildReqMsg(t, "") + + cmds, err := svc.processIncomingMDMCmds(ctx, enrolledDevice, reqMsg, RequestAuthStateTrusted) + require.NoError(t, err) + assert.True(t, hasGetForDevDetailSerial(cmds), "expected a Get for SMBIOSSerialNumber on an unlinked enrollment") + assert.False(t, ds.WindowsHostLiteByHardwareSerialFuncInvoked, "no Results present yet, so no host lookup expected") + }) + + t.Run("already-linked enrollment: no Get and no host lookup", func(t *testing.T) { + svc, ds, ctx := newSvc(t) + enrolledDevice := &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, HostUUID: testHostUUID} + reqMsg := buildReqMsg(t, "") + + cmds, err := svc.processIncomingMDMCmds(ctx, enrolledDevice, reqMsg, RequestAuthStateTrusted) + require.NoError(t, err) + assert.False(t, hasGetForDevDetailSerial(cmds), "linked enrollment must not inject a DevDetail Get") + assert.False(t, ds.WindowsHostLiteByHardwareSerialFuncInvoked) + assert.False(t, ds.UpdateMDMWindowsEnrollmentsHostUUIDFuncInvoked) + }) + + t.Run("Results with SMBIOS serial trigger linkage and skip the redundant Get", func(t *testing.T) { + svc, ds, ctx := newSvc(t) + enrolledDevice := &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, HostUUID: ""} + + ds.WindowsHostLiteByHardwareSerialFunc = func(_ context.Context, serial string) (*fleet.HostLite, error) { + assert.Equal(t, testSerial, serial) + return &fleet.HostLite{ID: testHostID, UUID: testHostUUID}, nil + } + ds.UpdateMDMWindowsEnrollmentsHostUUIDFunc = func(_ context.Context, hostUUID, mdmDeviceID string) (bool, error) { + assert.Equal(t, testHostUUID, hostUUID) + assert.Equal(t, testDeviceID, mdmDeviceID) + return true, nil + } + // Non-UPN enroll user means LinkWindowsHostMDMEnrollment skips the post-link UPN/SCIM bookkeeping. + ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFunc = func(_ context.Context, _ string) (*fleet.MDMWindowsEnrolledDevice, error) { + return &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, MDMEnrollUserID: ""}, nil + } + + extraBody := fmt.Sprintf(` + r1 + 1 + %s + + %s + %s + + `, uuid.NewString(), devDetailSMBIOSSerialNumberURI, testSerial) + reqMsg := buildReqMsg(t, extraBody) + + cmds, err := svc.processIncomingMDMCmds(ctx, enrolledDevice, reqMsg, RequestAuthStateTrusted) + require.NoError(t, err) + assert.True(t, ds.WindowsHostLiteByHardwareSerialFuncInvoked) + assert.True(t, ds.UpdateMDMWindowsEnrollmentsHostUUIDFuncInvoked) + assert.Equal(t, testHostUUID, enrolledDevice.HostUUID, "linkage should update in-memory HostUUID") + assert.False(t, hasGetForDevDetailSerial(cmds), "after successful linkage, no further Get should be injected") + }) + + t.Run("serial without matching host: Get is reinjected for retry", func(t *testing.T) { + svc, ds, ctx := newSvc(t) + enrolledDevice := &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, HostUUID: ""} + + ds.WindowsHostLiteByHardwareSerialFunc = func(_ context.Context, _ string) (*fleet.HostLite, error) { + return nil, sql.ErrNoRows + } + + extraBody := fmt.Sprintf(` + r1 + 1 + %s + + %s + %s + + `, uuid.NewString(), devDetailSMBIOSSerialNumberURI, testSerial) + reqMsg := buildReqMsg(t, extraBody) + + cmds, err := svc.processIncomingMDMCmds(ctx, enrolledDevice, reqMsg, RequestAuthStateTrusted) + require.NoError(t, err) + assert.True(t, ds.WindowsHostLiteByHardwareSerialFuncInvoked) + assert.False(t, ds.UpdateMDMWindowsEnrollmentsHostUUIDFuncInvoked) + assert.Empty(t, enrolledDevice.HostUUID, "no link means HostUUID stays empty") + assert.True(t, hasGetForDevDetailSerial(cmds), "without a host match, the Get is reinjected for the next session") + }) +} diff --git a/server/service/microsoft_mdm.go b/server/service/microsoft_mdm.go index 3ba1dfc48f5..10e5591fe09 100644 --- a/server/service/microsoft_mdm.go +++ b/server/service/microsoft_mdm.go @@ -34,6 +34,7 @@ import ( microsoft_mdm "github.com/fleetdm/fleet/v4/server/mdm/microsoft" "github.com/fleetdm/fleet/v4/server/mdm/microsoft/syncml" "github.com/fleetdm/fleet/v4/server/ptr" + "github.com/fleetdm/fleet/v4/server/service/osquery_utils" "github.com/fleetdm/fleet/v4/server/variables" mysql_driver "github.com/go-sql-driver/mysql" @@ -43,6 +44,12 @@ import ( const maxRequestLogSize = 10240 +// devDetailSMBIOSSerialNumberURI is the OMA-DM LocURI for the device's SMBIOS serial number. Used to link an unlinked +// Windows MDM enrollment (mdm_windows_enrollments.host_uuid = ”) to a Fleet host record without waiting for osquery's +// directIngestMDMDeviceIDWindows backstop. See LinkWindowsHostMDMEnrollment in server/service/osquery_utils for the +// linkage logic shared with the osquery path. +const devDetailSMBIOSSerialNumberURI = "./DevDetail/Ext/Microsoft/SMBIOSSerialNumber" + type SoapRequestContainer struct { Data *fleet.SoapRequest Params url.Values @@ -1496,10 +1503,6 @@ func (svc *Service) isFleetdPresentOnDevice(ctx context.Context, enrolledDevice return isPresent, nil } - // TODO: Add check here to determine if MDM DeviceID is connected with Smbios UUID present on - // host table. This new check should look into command results table and extract the value of - // ./DevDetail/Ext/Microsoft/SMBIOSSerialNumber for the given DeviceID and use that for hosts - // table lookup return true, nil } @@ -1695,6 +1698,56 @@ func (svc *Service) processIncomingAlertsCommands(ctx context.Context, messageID return nil } +// tryLinkUnlinkedEnrollmentFromDevDetail scans an incoming SyncML message for a Results command answering an earlier +// Get for ./DevDetail/Ext/Microsoft/SMBIOSSerialNumber, and if found, looks up the host by hardware_serial and links +// the enrollment to it. Returns true if a linkage was established (the in-memory enrolledDevice.HostUUID is updated so +// downstream callers in the same request see the linked state). +// +// Any error is non-fatal: this is the primary linkage path but osquery direct-ingest still runs as a backstop, and the +// Get is reinjected on every subsequent session until linkage succeeds. +func (svc *Service) tryLinkUnlinkedEnrollmentFromDevDetail(ctx context.Context, enrolledDevice *fleet.MDMWindowsEnrolledDevice, reqMsg *fleet.SyncML) bool { + if reqMsg == nil { + return false + } + var serial string + for _, op := range reqMsg.GetOrderedCmds() { + if op.Verb != mdm_types.CmdResults || len(op.Cmd.Items) == 0 { + continue + } + item := op.Cmd.Items[0] + if item.Source == nil || *item.Source != devDetailSMBIOSSerialNumberURI { + continue + } + if item.Data == nil { + continue + } + serial = strings.TrimSpace(item.Data.Content) + break + } + if serial == "" { + return false + } + host, err := svc.ds.WindowsHostLiteByHardwareSerial(ctx, serial) + if err != nil { + if !fleet.IsNotFound(err) { + svc.logger.WarnContext(ctx, "windows mdm: host lookup by serial failed", + "err", err, "device_id", enrolledDevice.MDMDeviceID) + } + // NotFound means the host hasn't enrolled in osquery yet; we'll retry next session. + return false + } + updated, err := osquery_utils.LinkWindowsHostMDMEnrollment(ctx, svc.logger, svc.ds, host.ID, host.UUID, enrolledDevice.MDMDeviceID) + if err != nil { + svc.logger.WarnContext(ctx, "windows mdm: link by DevDetail failed", + "err", err, "device_id", enrolledDevice.MDMDeviceID) + return false + } + if updated { + enrolledDevice.HostUUID = host.UUID + } + return updated +} + // processIncomingMDMCmds process the incoming message from the device // It will return the list of operations that need to be sent to the device. // enrolledDevice is the enrollment resolved upstream by isTrustedRequest and @@ -1801,6 +1854,14 @@ func (svc *Service) processIncomingMDMCmds(ctx context.Context, enrolledDevice * responseCmds = append(responseCmds, ackMsg) } + // If this enrollment isn't linked yet, try to link it using a DevDetail/SMBIOSSerialNumber Result the device may + // have included in this message (in response to a Get we sent during a previous session). If the link succeeds, + // enrolledDevice.HostUUID is updated in memory so downstream callers (ESP coordination, saveResponse, etc.) in this + // same request see the linked state instead of waiting for the next session. + if enrolledDevice.HostUUID == "" { + svc.tryLinkUnlinkedEnrollmentFromDevDetail(ctx, enrolledDevice, reqMsg) + } + // List of CmdRef that need to be re-issued as commands // However it's a list of nested Command IDs, and not something we can use directly for command_uuid in windows_mdm_commands alreadyExistsCmdIDs := []string{} @@ -1842,6 +1903,15 @@ func (svc *Service) processIncomingMDMCmds(ctx context.Context, enrolledDevice * return nil, err } + // If this enrollment is still unlinked after processing the incoming message, ask the device for its SMBIOS serial + // on the next round-trip. This is the primary linkage mechanism: it lets the server populate + // mdm_windows_enrollments.host_uuid in one SyncML round-trip instead of waiting for osquery's distributed-read + // cycle (~10s) to backfill via directIngestMDMDeviceIDWindows. The Get is idempotent and reinjected each session + // until linkage succeeds; osquery direct-ingest remains as a backstop for hosts that never reply to DevDetail. + if enrolledDevice.HostUUID == "" { + responseCmds = append(responseCmds, newSyncMLCmdGet(devDetailSMBIOSSerialNumberURI)) + } + return responseCmds, nil } @@ -2784,10 +2854,10 @@ func (svc *Service) storeWindowsMDMEnrolledDevice(ctx context.Context, userID st return err } - // TODO: azure enrollments come with an empty uuid, I haven't figured - // out a good way to identify the device here. - // Note that we currently do the Enrollment->Host mapping during the next - // refetch of the host + // For Azure (automatic) enrollments, hostUUID is empty here because the WSTEP RequestSecurityToken does not carry + // any identifier that maps to hosts.uuid. The enrollment row is inserted unlinked; processIncomingMDMCmds asks the + // device for its SMBIOS serial number on the first management session and links the row when the device replies. + // osquery's directIngestMDMDeviceIDWindows remains as a backstop. displayName := reqDeviceName var serial string if hostUUID != "" { @@ -3183,6 +3253,16 @@ func newSyncMLCmdNode(cmdVerb string, cmdTarget string) *mdm_types.SyncMLCmd { return newSyncMLCmdWithItem(&cmdVerb, nil, item) } +// newSyncMLCmdGet creates a SyncML Get command targeting the given OMA-DM LocURI. Get commands have no data, format, or +// type on the request; the device fills those in on the corresponding Results response. +func newSyncMLCmdGet(cmdTarget string) *mdm_types.SyncMLCmd { + verb := fleet.CmdGet + item := newSyncMLItem(nil, &cmdTarget, nil, nil, nil) + cmd := newSyncMLCmdWithItem(&verb, nil, item) + cmd.CmdID = mdm_types.CmdID{Value: uuid.New().String()} + return cmd +} + // newSyncMLCmdInt creates a new SyncML command with text data func newSyncMLCmdInt(cmdVerb string, cmdTarget string, cmdDataValue string) *mdm_types.SyncMLCmd { cmdType := "text/plain" diff --git a/server/service/osquery_utils/queries.go b/server/service/osquery_utils/queries.go index efceac92d32..932096b8008 100644 --- a/server/service/osquery_utils/queries.go +++ b/server/service/osquery_utils/queries.go @@ -3036,58 +3036,70 @@ func directIngestMDMDeviceIDWindows(ctx context.Context, logger *slog.Logger, ho if len(rows) > 1 { return ctxerr.Errorf(ctx, "directIngestMDMDeviceIDWindows invalid number of rows: %d", len(rows)) } - updated, err := ds.UpdateMDMWindowsEnrollmentsHostUUID(ctx, host.UUID, rows[0]["data"]) + _, err := LinkWindowsHostMDMEnrollment(ctx, logger, ds, host.ID, host.UUID, rows[0]["data"]) + return err +} + +// LinkWindowsHostMDMEnrollment associates the Windows MDM enrollment for mdmDeviceID with the host identified by +// (hostID, hostUUID). On the first time the linkage is established (i.e. mdm_windows_enrollments.host_uuid changes), it +// also reconciles the host's IDP device mapping, SCIM user attribution, and DEP flag for Azure (Entra) enrollments, +// matching the post-link bookkeeping that osquery's directIngestMDMDeviceIDWindows has historically performed. +// +// Returns true if the row was updated (first linkage or re-linkage to a different host), false if the enrollment was +// already linked to this host. Callers should not treat false as an error. +// +// This helper is shared by the osquery direct-ingest path (legacy) and the OMA-DM DevDetail SyncML path (primary), so +// both linkage triggers run the same post-link bookkeeping exactly once per linkage. +func LinkWindowsHostMDMEnrollment(ctx context.Context, logger *slog.Logger, ds fleet.Datastore, hostID uint, hostUUID, mdmDeviceID string) (bool, error) { + updated, err := ds.UpdateMDMWindowsEnrollmentsHostUUID(ctx, hostUUID, mdmDeviceID) if err != nil { - return ctxerr.Wrap(ctx, err, "updating windows mdm device id") + return false, ctxerr.Wrap(ctx, err, "updating windows mdm device id") } - if updated { - device, err := ds.MDMWindowsGetEnrolledDeviceWithDeviceID(ctx, rows[0]["data"]) - if err != nil { - return ctxerr.Wrap(ctx, err, "getting windows mdm device after updating host uuid") + if !updated { + return false, nil + } + device, err := ds.MDMWindowsGetEnrolledDeviceWithDeviceID(ctx, mdmDeviceID) + if err != nil { + return updated, ctxerr.Wrap(ctx, err, "getting windows mdm device after updating host uuid") + } + if device == nil || !microsoft_mdm.IsValidUPN(device.MDMEnrollUserID) { + return updated, nil + } + // Update the host's MDM enrolled flags to show it as a manual enrollment so it doesn't take two full refreshes to + // reflect this state. + if device.MDMNotInOOBE { + if err := ds.UpdateMDMInstalledFromDEP(ctx, hostID, false); err != nil { + return updated, ctxerr.Wrap(ctx, err, "updating windows mdm installed from dep flag") } - if device != nil && microsoft_mdm.IsValidUPN(device.MDMEnrollUserID) { - // Update the host's MDM enrolled flags to show it as a manual enrollment. THis is to avoid - // it taking two full refreshes to show this - if device.MDMNotInOOBE { - err = ds.UpdateMDMInstalledFromDEP(ctx, host.ID, false) - if err != nil { - return ctxerr.Wrap(ctx, err, "updating windows mdm installed from dep flag") - } - } - - mapping := []*fleet.HostDeviceMapping{ - { - HostID: host.ID, - Email: device.MDMEnrollUserID, - Source: fleet.DeviceMappingMDMIdpAccounts, - }, - } - err = ds.ReplaceHostDeviceMapping(ctx, host.ID, mapping, fleet.DeviceMappingMDMIdpAccounts) - if err != nil { - return ctxerr.Wrap(ctx, err, "replacing host device mapping for windows mdm enrolled device") - } - // Check if the user is a valid SCIM user to manage the join table - scimUser, err := ds.ScimUserByUserNameOrEmail(ctx, device.MDMEnrollUserID, device.MDMEnrollUserID) - if err != nil && !fleet.IsNotFound(err) && err != sql.ErrNoRows { - return ctxerr.Wrap(ctx, err, "find SCIM user for Windows Azure enrollment linking by username or email") - } - if err == nil && scimUser != nil { - // User exists in SCIM, create/update the mapping for additional attributes - // This enables fields like idp_full_name, idp_groups, etc. to appear in the API - if err := ds.SetOrUpdateHostSCIMUserMapping(ctx, host.ID, scimUser.ID); err != nil { - // Log the error but don't fail the request since the main IDP mapping succeeded - logger.DebugContext(ctx, "failed to set SCIM user mapping", "err", err) - } - } else { - // User doesn't exist in SCIM, remove any existing SCIM mapping for this host - if err := ds.DeleteHostSCIMUserMapping(ctx, host.ID); err != nil && !fleet.IsNotFound(err) { - // Log the error but don't fail the request - logger.DebugContext(ctx, "failed to delete SCIM user mapping", "err", err) - } - } + } + mapping := []*fleet.HostDeviceMapping{ + { + HostID: hostID, + Email: device.MDMEnrollUserID, + Source: fleet.DeviceMappingMDMIdpAccounts, + }, + } + if err := ds.ReplaceHostDeviceMapping(ctx, hostID, mapping, fleet.DeviceMappingMDMIdpAccounts); err != nil { + return updated, ctxerr.Wrap(ctx, err, "replacing host device mapping for windows mdm enrolled device") + } + // Check if the user is a valid SCIM user to manage the join table. + scimUser, err := ds.ScimUserByUserNameOrEmail(ctx, device.MDMEnrollUserID, device.MDMEnrollUserID) + if err != nil && !fleet.IsNotFound(err) && err != sql.ErrNoRows { + return updated, ctxerr.Wrap(ctx, err, "find SCIM user for Windows Azure enrollment linking by username or email") + } + if err == nil && scimUser != nil { + // User exists in SCIM, create/update the mapping for additional attributes (idp_full_name, idp_groups, etc.). + if err := ds.SetOrUpdateHostSCIMUserMapping(ctx, hostID, scimUser.ID); err != nil { + // Log the error but don't fail the linkage since the main IDP mapping succeeded. + logger.DebugContext(ctx, "failed to set SCIM user mapping", "err", err) + } + } else { + // User doesn't exist in SCIM, remove any existing SCIM mapping for this host. + if err := ds.DeleteHostSCIMUserMapping(ctx, hostID); err != nil && !fleet.IsNotFound(err) { + logger.DebugContext(ctx, "failed to delete SCIM user mapping", "err", err) } } - return nil + return updated, nil } var luksVerifyQuery = DetailQuery{ From cfd8fc36095eae71267cc82e389c7a58d20d4655 Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky <2685025+getvictor@users.noreply.github.com> Date: Wed, 27 May 2026 17:50:04 +0000 Subject: [PATCH 2/7] windows_mdm: address AI review feedback on DevDetail linkage - Always refresh enrolledDevice.HostUUID after a successful host lookup + link, even when UpdateMDMWindowsEnrollmentsHostUUID reports updated=false. The "no change" outcome from UpdateMDMWindowsEnrollmentsHostUUID also covers the case where the row was already linked to the same hostUUID by a concurrent path, so the in-memory enrolledDevice must still pick up the linkage to avoid reinjecting a redundant DevDetail Get for the rest of the request. - Scan every Item in every Results command, not just Items[0]. SyncML Results can carry multiple items, so the SMBIOS serial may not be the first one. - Use ds.writer(ctx) for WindowsHostLiteByHardwareSerial. The lookup runs immediately after osquery's enroll write, and replica lag was producing false NotFound results that delayed linkage. - Clarify LinkWindowsHostMDMEnrollment docstring: updated=false also covers "no row matched mdmDeviceID", not only "already linked to this host". - Replace curly quotes with ASCII '' in two comments. Adds two regression tests covering the multi-item Results case and the updated=false in-memory refresh path. --- server/datastore/mysql/microsoft_mdm.go | 5 +- server/service/mdm_test.go | 67 +++++++++++++++++++++++++ server/service/microsoft_mdm.go | 33 +++++++----- server/service/osquery_utils/queries.go | 7 ++- 4 files changed, 97 insertions(+), 15 deletions(-) diff --git a/server/datastore/mysql/microsoft_mdm.go b/server/datastore/mysql/microsoft_mdm.go index 69566a43b80..8969518bcc5 100644 --- a/server/datastore/mysql/microsoft_mdm.go +++ b/server/datastore/mysql/microsoft_mdm.go @@ -204,7 +204,10 @@ func (ds *Datastore) WindowsHostLiteByHardwareSerial(ctx context.Context, hardwa WHERE h.hardware_serial = ? AND h.platform = 'windows' LIMIT 2` var hosts []*fleet.HostLite - if err := sqlx.SelectContext(ctx, ds.reader(ctx), &hosts, stmt, hardwareSerial); err != nil { + // Read from the primary: the host row may have been written seconds ago (osquery enroll → directIngestMDM + // completes the host insert just before the first SyncML management session arrives). Replica lag here returns a + // false NotFound and the linkage waits another session, defeating the point of this path. + if err := sqlx.SelectContext(ctx, ds.writer(ctx), &hosts, stmt, hardwareSerial); err != nil { return nil, ctxerr.Wrap(ctx, err, "select windows host by hardware serial") } if len(hosts) != 1 { diff --git a/server/service/mdm_test.go b/server/service/mdm_test.go index a356b08df25..f6ce5b5291d 100644 --- a/server/service/mdm_test.go +++ b/server/service/mdm_test.go @@ -3310,4 +3310,71 @@ func TestProcessIncomingMDMCmdsDevDetailLinkage(t *testing.T) { assert.Empty(t, enrolledDevice.HostUUID, "no link means HostUUID stays empty") assert.True(t, hasGetForDevDetailSerial(cmds), "without a host match, the Get is reinjected for the next session") }) + + t.Run("SMBIOSSerialNumber Item is not the first one: still found", func(t *testing.T) { + svc, ds, ctx := newSvc(t) + enrolledDevice := &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, HostUUID: ""} + + ds.WindowsHostLiteByHardwareSerialFunc = func(_ context.Context, serial string) (*fleet.HostLite, error) { + assert.Equal(t, testSerial, serial) + return &fleet.HostLite{ID: testHostID, UUID: testHostUUID}, nil + } + ds.UpdateMDMWindowsEnrollmentsHostUUIDFunc = func(_ context.Context, _, _ string) (bool, error) { + return true, nil + } + ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFunc = func(_ context.Context, _ string) (*fleet.MDMWindowsEnrolledDevice, error) { + return &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, MDMEnrollUserID: ""}, nil + } + + // The serial is the second Item in the Results command, not the first. A naive Items[0]-only scan misses it. + extraBody := fmt.Sprintf(` + r1 + 1 + %s + + ./DevDetail/Ext/Microsoft/DeviceName + DESKTOP-OTHER + + + %s + %s + + `, uuid.NewString(), devDetailSMBIOSSerialNumberURI, testSerial) + reqMsg := buildReqMsg(t, extraBody) + + cmds, err := svc.processIncomingMDMCmds(ctx, enrolledDevice, reqMsg, RequestAuthStateTrusted) + require.NoError(t, err) + assert.Equal(t, testHostUUID, enrolledDevice.HostUUID) + assert.False(t, hasGetForDevDetailSerial(cmds)) + }) + + t.Run("link already applied by another path: HostUUID still refreshed in-memory", func(t *testing.T) { + svc, ds, ctx := newSvc(t) + enrolledDevice := &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, HostUUID: ""} + + ds.WindowsHostLiteByHardwareSerialFunc = func(_ context.Context, _ string) (*fleet.HostLite, error) { + return &fleet.HostLite{ID: testHostID, UUID: testHostUUID}, nil + } + // updated=false simulates the row already holding host_uuid=testHostUUID (the WHERE host_uuid <> ? guard + // short-circuited). In-memory state must still be refreshed so no redundant Get is sent. + ds.UpdateMDMWindowsEnrollmentsHostUUIDFunc = func(_ context.Context, _, _ string) (bool, error) { + return false, nil + } + + extraBody := fmt.Sprintf(` + r1 + 1 + %s + + %s + %s + + `, uuid.NewString(), devDetailSMBIOSSerialNumberURI, testSerial) + reqMsg := buildReqMsg(t, extraBody) + + cmds, err := svc.processIncomingMDMCmds(ctx, enrolledDevice, reqMsg, RequestAuthStateTrusted) + require.NoError(t, err) + assert.Equal(t, testHostUUID, enrolledDevice.HostUUID, "even updated=false must refresh in-memory HostUUID") + assert.False(t, hasGetForDevDetailSerial(cmds), "in-memory HostUUID is now set; no redundant Get") + }) } diff --git a/server/service/microsoft_mdm.go b/server/service/microsoft_mdm.go index 10e5591fe09..c657bad32dd 100644 --- a/server/service/microsoft_mdm.go +++ b/server/service/microsoft_mdm.go @@ -1709,20 +1709,27 @@ func (svc *Service) tryLinkUnlinkedEnrollmentFromDevDetail(ctx context.Context, if reqMsg == nil { return false } + // A SyncML Results command can carry multiple Items; the device may include the SMBIOSSerialNumber alongside other + // DevDetail values in a single response. Scan every item in every Results command, not just Items[0], or a serial + // returned in a later position is missed and the Get gets reinjected forever. var serial string +scan: for _, op := range reqMsg.GetOrderedCmds() { - if op.Verb != mdm_types.CmdResults || len(op.Cmd.Items) == 0 { + if op.Verb != mdm_types.CmdResults { continue } - item := op.Cmd.Items[0] - if item.Source == nil || *item.Source != devDetailSMBIOSSerialNumberURI { - continue - } - if item.Data == nil { - continue + for _, item := range op.Cmd.Items { + if item.Source == nil || *item.Source != devDetailSMBIOSSerialNumberURI { + continue + } + if item.Data == nil { + continue + } + serial = strings.TrimSpace(item.Data.Content) + if serial != "" { + break scan + } } - serial = strings.TrimSpace(item.Data.Content) - break } if serial == "" { return false @@ -1742,9 +1749,11 @@ func (svc *Service) tryLinkUnlinkedEnrollmentFromDevDetail(ctx context.Context, "err", err, "device_id", enrolledDevice.MDMDeviceID) return false } - if updated { - enrolledDevice.HostUUID = host.UUID - } + // Always refresh in-memory HostUUID after a successful link attempt, even when updated==false. updated=false means + // UpdateMDMWindowsEnrollmentsHostUUID's `WHERE host_uuid <> ?` guard short-circuited because the DB row already + // holds this host's UUID (e.g. another concurrent path linked it, or the in-memory row predates a writer-side + // update). Without this refresh the rest of the request would still see HostUUID="" and reinject another Get. + enrolledDevice.HostUUID = host.UUID return updated } diff --git a/server/service/osquery_utils/queries.go b/server/service/osquery_utils/queries.go index 932096b8008..83f1fa5b2ee 100644 --- a/server/service/osquery_utils/queries.go +++ b/server/service/osquery_utils/queries.go @@ -3045,8 +3045,11 @@ func directIngestMDMDeviceIDWindows(ctx context.Context, logger *slog.Logger, ho // also reconciles the host's IDP device mapping, SCIM user attribution, and DEP flag for Azure (Entra) enrollments, // matching the post-link bookkeeping that osquery's directIngestMDMDeviceIDWindows has historically performed. // -// Returns true if the row was updated (first linkage or re-linkage to a different host), false if the enrollment was -// already linked to this host. Callers should not treat false as an error. +// Returns true when UpdateMDMWindowsEnrollmentsHostUUID actually changed the row, false when it did not. The "no +// change" case covers two scenarios callers must not conflate with an error: (a) the enrollment was already linked to +// this same hostUUID, so the `WHERE host_uuid <> ?` guard short-circuited; (b) no row matched mdmDeviceID at all (e.g. +// the enrollment was deleted concurrently). Callers that depend on linkage being applied should re-read the enrollment +// rather than infer it from the boolean alone. // // This helper is shared by the osquery direct-ingest path (legacy) and the OMA-DM DevDetail SyncML path (primary), so // both linkage triggers run the same post-link bookkeeping exactly once per linkage. From 3d10cc6a84a63bf1fe39b603db85aa8652230966 Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky <2685025+getvictor@users.noreply.github.com> Date: Wed, 27 May 2026 18:32:11 +0000 Subject: [PATCH 3/7] windows_mdm: suppress unmatched-command warning for DevDetail link probe The DevDetail/SMBIOSSerialNumber Get injected by processIncomingMDMCmds for unlinked enrollments is never inserted into windows_mdm_commands because it is a protocol-level linkage probe, not a queued command. With a fresh UUID CmdID each session, the device's Status/Results reply on the next session left MDMWindowsSaveResponse logging "unmatched Windows MDM commands" once per session until linkage completed, since BYOD enrollments in the unlinked window typically have no other pending commands to suppress the warning. Use a stable fleet-internal CmdID prefix ("fleet-internal-") for these server-injected probes. MDMWindowsSaveResponse now filters CmdRefs with that prefix out before deciding whether to warn, so a properly functioning linkage probe is silent and real unmatched-command alerts remain visible. Cross-session correlation is now also possible since the CmdID is stable. No DB schema or windows_mdm_commands changes; the probe still lives only in the SyncML envelope and the message-level audit in windows_mdm_responses. --- server/datastore/mysql/microsoft_mdm.go | 14 ++++++++++++-- server/fleet/microsoft_mdm.go | 13 +++++++++++++ server/fleet/microsoft_mdm_test.go | 18 ++++++++++++++++++ server/service/mdm_test.go | 12 ++++++++++++ server/service/microsoft_mdm.go | 9 ++++++++- 5 files changed, 63 insertions(+), 3 deletions(-) diff --git a/server/datastore/mysql/microsoft_mdm.go b/server/datastore/mysql/microsoft_mdm.go index 8969518bcc5..5eb99e31e2c 100644 --- a/server/datastore/mysql/microsoft_mdm.go +++ b/server/datastore/mysql/microsoft_mdm.go @@ -845,9 +845,19 @@ func (ds *Datastore) MDMWindowsSaveResponse(ctx context.Context, enrolledDevice } if len(matchingCmds) == 0 { - if len(commandIDsBeingResent) == 0 { + // Suppress the warning when the only CmdRefs in the message are Fleet-internal commands that are + // intentionally absent from windows_mdm_commands (e.g. the DevDetail/SMBIOSSerialNumber Get used for + // unlinked-enrollment linkage). Without this filter, an unlinked Windows enrollment with no other pending + // commands warns on every session until linkage completes. + externalCmdRefs := make([]string, 0, len(enrichedSyncML.CmdRefUUIDs)) + for _, u := range enrichedSyncML.CmdRefUUIDs { + if !fleet.IsFleetInternalCmdID(u) { + externalCmdRefs = append(externalCmdRefs, u) + } + } + if len(commandIDsBeingResent) == 0 && len(externalCmdRefs) > 0 { // Only log if not resending commands as we then can expect no matching commands - ds.logger.WarnContext(ctx, "unmatched Windows MDM commands", "uuids", strings.Join(enrichedSyncML.CmdRefUUIDs, ","), "mdm_device_id", + ds.logger.WarnContext(ctx, "unmatched Windows MDM commands", "uuids", strings.Join(externalCmdRefs, ","), "mdm_device_id", enrolledDevice.MDMDeviceID) } return nil diff --git a/server/fleet/microsoft_mdm.go b/server/fleet/microsoft_mdm.go index 19c7a42504f..a631a965e5f 100644 --- a/server/fleet/microsoft_mdm.go +++ b/server/fleet/microsoft_mdm.go @@ -1012,6 +1012,19 @@ const ( CmdStatus = "Status" // Protocol Command verb Status ) +// FleetInternalCmdIDPrefix marks Fleet-injected SyncML commands that are sent inline in a management response but never +// persisted to windows_mdm_commands. The device's / for these commands will reference a CmdID that +// won't match any row in windows_mdm_commands. MDMWindowsSaveResponse uses this prefix to skip them before warning +// about "unmatched Windows MDM commands". Example: the DevDetail/SMBIOSSerialNumber Get used to link unlinked Windows +// MDM enrollments (see server/service/microsoft_mdm.go). +const FleetInternalCmdIDPrefix = "fleet-internal-" + +// IsFleetInternalCmdID reports whether a SyncML CmdID was injected by Fleet itself and is intentionally absent from +// windows_mdm_commands. +func IsFleetInternalCmdID(cmdID string) bool { + return strings.HasPrefix(cmdID, FleetInternalCmdIDPrefix) +} + // ProtoCmdOperation is the abstraction to represent a SyncML Protocol Command type ProtoCmdOperation struct { Verb string `db:"verb"` diff --git a/server/fleet/microsoft_mdm_test.go b/server/fleet/microsoft_mdm_test.go index f84fb563e94..cc54ebe99eb 100644 --- a/server/fleet/microsoft_mdm_test.go +++ b/server/fleet/microsoft_mdm_test.go @@ -812,3 +812,21 @@ func TestExtractLocURIsFromProfileBytes(t *testing.T) { require.Equal(t, []string{"./Device/A"}, uris) }) } + +func TestIsFleetInternalCmdID(t *testing.T) { + for _, tc := range []struct { + name string + in string + want bool + }{ + {"empty string", "", false}, + {"random UUID", "550e8400-e29b-41d4-a716-446655440000", false}, + {"unrelated prefix", "foo-internal-bar", false}, + {"prefix only", FleetInternalCmdIDPrefix, true}, + {"devdetail link probe", FleetInternalCmdIDPrefix + "devdetail-smbios-serial", true}, + } { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.want, IsFleetInternalCmdID(tc.in)) + }) + } +} diff --git a/server/service/mdm_test.go b/server/service/mdm_test.go index f6ce5b5291d..80f4ddb377d 100644 --- a/server/service/mdm_test.go +++ b/server/service/mdm_test.go @@ -3233,6 +3233,18 @@ func TestProcessIncomingMDMCmdsDevDetailLinkage(t *testing.T) { require.NoError(t, err) assert.True(t, hasGetForDevDetailSerial(cmds), "expected a Get for SMBIOSSerialNumber on an unlinked enrollment") assert.False(t, ds.WindowsHostLiteByHardwareSerialFuncInvoked, "no Results present yet, so no host lookup expected") + // The injected Get must use a stable fleet-internal CmdID so MDMWindowsSaveResponse can suppress the + // "unmatched Windows MDM commands" warning when the device replies on the next session. + var foundInternalCmdID bool + for _, c := range cmds { + if c.XMLName.Local == fleet.CmdGet && len(c.Items) > 0 && c.Items[0].Target != nil && + *c.Items[0].Target == devDetailSMBIOSSerialNumberURI { + assert.True(t, fleet.IsFleetInternalCmdID(c.CmdID.Value), + "CmdID %q must use the fleet-internal sentinel prefix", c.CmdID.Value) + foundInternalCmdID = true + } + } + assert.True(t, foundInternalCmdID, "expected to find the DevDetail Get among response commands") }) t.Run("already-linked enrollment: no Get and no host lookup", func(t *testing.T) { diff --git a/server/service/microsoft_mdm.go b/server/service/microsoft_mdm.go index c657bad32dd..ade5115cd22 100644 --- a/server/service/microsoft_mdm.go +++ b/server/service/microsoft_mdm.go @@ -1917,8 +1917,15 @@ func (svc *Service) processIncomingMDMCmds(ctx context.Context, enrolledDevice * // mdm_windows_enrollments.host_uuid in one SyncML round-trip instead of waiting for osquery's distributed-read // cycle (~10s) to backfill via directIngestMDMDeviceIDWindows. The Get is idempotent and reinjected each session // until linkage succeeds; osquery direct-ingest remains as a backstop for hosts that never reply to DevDetail. + // + // The Get uses a stable fleet-internal CmdID instead of a fresh UUID so that MDMWindowsSaveResponse can recognize + // and skip it when checking for "unmatched Windows MDM commands". The Get is never inserted into windows_mdm_commands + // (it's purely a protocol-level linkage probe), so without that filter the device's Status/Results reply would log + // a warning on every session for unlinked enrollments. if enrolledDevice.HostUUID == "" { - responseCmds = append(responseCmds, newSyncMLCmdGet(devDetailSMBIOSSerialNumberURI)) + get := newSyncMLCmdGet(devDetailSMBIOSSerialNumberURI) + get.CmdID = mdm_types.CmdID{Value: fleet.FleetInternalCmdIDPrefix + "devdetail-smbios-serial"} + responseCmds = append(responseCmds, get) } return responseCmds, nil From 809a82310153364fb50a17b470b48bdec8b581fd Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky <2685025+getvictor@users.noreply.github.com> Date: Thu, 28 May 2026 22:38:25 +0000 Subject: [PATCH 4/7] Code review fixes --- server/datastore/mysql/hosts.go | 29 ++-- server/datastore/mysql/microsoft_mdm.go | 29 +--- server/fleet/datastore.go | 5 +- server/fleet/hosts.go | 51 ++++++ server/fleet/hosts_test.go | 59 +++++++ server/service/mdm_test.go | 196 +++++++++++++----------- server/service/microsoft_mdm.go | 48 +++--- server/service/osquery_utils/queries.go | 3 - 8 files changed, 265 insertions(+), 155 deletions(-) diff --git a/server/datastore/mysql/hosts.go b/server/datastore/mysql/hosts.go index e0ee5d4d8f0..6db70c9f9c8 100644 --- a/server/datastore/mysql/hosts.go +++ b/server/datastore/mysql/hosts.go @@ -6338,6 +6338,21 @@ func (ds *Datastore) HostLiteByID(ctx context.Context, id uint) (*fleet.HostLite return ds.loadHostLite(ctx, &id, nil) } +// hostLiteColumns is the shared SELECT column list +const hostLiteColumns = ` + h.id, + h.team_id, + h.osquery_host_id, + COALESCE(h.node_key, '') AS node_key, + h.computer_name, + h.hostname, + h.uuid, + h.hardware_model, + h.hardware_serial, + h.distributed_interval, + h.config_tls_refresh, + COALESCE(hst.seen_time, h.created_at) AS seen_time` + func (ds *Datastore) loadHostLite(ctx context.Context, id *uint, identifier *string) (*fleet.HostLite, error) { if id == nil && identifier == nil { return nil, errors.New("must set one of id or identifier") @@ -6346,19 +6361,7 @@ func (ds *Datastore) loadHostLite(ctx context.Context, id *uint, identifier *str return nil, errors.New("cannot set both id and identifier") } stmt := ` - SELECT - h.id, - h.team_id, - h.osquery_host_id, - COALESCE(h.node_key, '') AS node_key, - h.computer_name, - h.hostname, - h.uuid, - h.hardware_model, - h.hardware_serial, - h.distributed_interval, - h.config_tls_refresh, - COALESCE(hst.seen_time, h.created_at) AS seen_time + SELECT ` + hostLiteColumns + ` FROM hosts h LEFT JOIN host_seen_times hst ON (h.id = hst.host_id) %s diff --git a/server/datastore/mysql/microsoft_mdm.go b/server/datastore/mysql/microsoft_mdm.go index 5eb99e31e2c..8ac1aa2e135 100644 --- a/server/datastore/mysql/microsoft_mdm.go +++ b/server/datastore/mysql/microsoft_mdm.go @@ -176,38 +176,23 @@ func (ds *Datastore) MDMWindowsGetUnlinkedEnrolledDeviceWithDeviceName(ctx conte return &winMDMDevice, nil } -// WindowsHostLiteByHardwareSerial looks up a Windows host by its hardware_serial. It is used to link an unlinked Windows -// MDM enrollment (mdm_windows_enrollments.host_uuid = ”) to its host using DevDetail/Ext/Microsoft/SMBIOSSerialNumber -// reported during the first SyncML management session. The platform filter avoids cross-platform collisions on shared -// serials. If multiple Windows hosts share the same serial (e.g. VM gold images that did not regenerate SMBIOS), the -// caller cannot pick safely so we return NotFound and let the osquery direct-ingest backstop resolve linkage later. +// WindowsHostLiteByHardwareSerial looks up a Windows host by its hardware_serial. If multiple Windows hosts share the +// same serial (e.g. VM gold images that did not regenerate SMBIOS), the caller cannot pick safely so we return NotFound. +// +// The read honors ctxdb.RequirePrimary: a caller linking a just-enrolled host (whose hosts row may have been inserted +// seconds ago) must pass a primary-required context, otherwise replica lag can return a false NotFound. func (ds *Datastore) WindowsHostLiteByHardwareSerial(ctx context.Context, hardwareSerial string) (*fleet.HostLite, error) { if hardwareSerial == "" { return nil, ctxerr.Wrap(ctx, notFound("Host").WithMessage("empty hardware serial")) } const stmt = ` - SELECT - h.id, - h.team_id, - h.osquery_host_id, - COALESCE(h.node_key, '') AS node_key, - h.computer_name, - h.hostname, - h.uuid, - h.hardware_model, - h.hardware_serial, - h.distributed_interval, - h.config_tls_refresh, - COALESCE(hst.seen_time, h.created_at) AS seen_time + SELECT ` + hostLiteColumns + ` FROM hosts h LEFT JOIN host_seen_times hst ON h.id = hst.host_id WHERE h.hardware_serial = ? AND h.platform = 'windows' LIMIT 2` var hosts []*fleet.HostLite - // Read from the primary: the host row may have been written seconds ago (osquery enroll → directIngestMDM - // completes the host insert just before the first SyncML management session arrives). Replica lag here returns a - // false NotFound and the linkage waits another session, defeating the point of this path. - if err := sqlx.SelectContext(ctx, ds.writer(ctx), &hosts, stmt, hardwareSerial); err != nil { + if err := sqlx.SelectContext(ctx, ds.reader(ctx), &hosts, stmt, hardwareSerial); err != nil { return nil, ctxerr.Wrap(ctx, err, "select windows host by hardware serial") } if len(hosts) != 1 { diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index 9740bb67c6f..5b29c1fe30c 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -2111,10 +2111,7 @@ type Datastore interface { // find a row because the host->enrollment link has not been resolved yet. MDMWindowsGetUnlinkedEnrolledDeviceWithDeviceName(ctx context.Context, deviceName string) (*MDMWindowsEnrolledDevice, error) - // WindowsHostLiteByHardwareSerial returns a HostLite for the Windows host whose hardware_serial matches the given - // serial. Used to link an unlinked Windows MDM enrollment to a host using SMBIOS data reported via OMA-DM DevDetail - // before osquery's directIngestMDMDeviceIDWindows has run. Returns NotFound if no Windows host matches or if multiple - // Windows hosts share the same serial (which would make the linkage ambiguous). + // WindowsHostLiteByHardwareSerial returns a HostLite for the Windows host whose hardware_serial matches the given serial. WindowsHostLiteByHardwareSerial(ctx context.Context, hardwareSerial string) (*HostLite, error) // MDMWindowsDeleteEnrolledDeviceWithDeviceID deletes a give MDMWindowsEnrolledDevice entry from the database using the device id diff --git a/server/fleet/hosts.go b/server/fleet/hosts.go index f6d2563422a..d536bbcc53d 100644 --- a/server/fleet/hosts.go +++ b/server/fleet/hosts.go @@ -57,6 +57,57 @@ func (s HostStatus) IsValid() bool { } } +// placeholderHardwareSerials is the set of well-known junk SMBIOS serial numbers that OEM/BIOS firmware and VM +// templates emit in place of a real, unique serial. Keys are already trimmed and lower-cased; compare against a +// normalized serial. The list is best-effort and will never be exhaustive, so IsPlaceholderHardwareSerial also applies +// a repeated-character heuristic and callers fall back to a unique identifier when a serial is a placeholder. +var placeholderHardwareSerials = map[string]struct{}{ + "to be filled by o.e.m.": {}, + "default string": {}, + "system serial number": {}, + "not specified": {}, + "not applicable": {}, + "none": {}, + "oem": {}, + "o.e.m.": {}, + "default": {}, + "unknown": {}, + "chassis serial number": {}, + "base board serial number": {}, + "baseboard serial number": {}, + "123456789": {}, + "0123456789": {}, + "1234567890": {}, + "1234567": {}, + "n/a": {}, + "na": {}, + "invalid": {}, +} + +// IsPlaceholderHardwareSerial reports whether serial is empty or a well-known placeholder/junk value that does not +// uniquely identify a device (common on whitebox/consumer hardware and un-sysprepped VM templates). Callers must not +// use such a serial to match or link a host, since multiple unrelated devices report the same value; they should fall +// back to an unambiguous identifier instead. +// +// Matching is case-insensitive and trimmed. In addition to the known-value set, a serial made up of a single repeated +// character (e.g. "0", "00000000", "xxxxxxx", "-------") is treated as a placeholder, since those cannot be enumerated. +func IsPlaceholderHardwareSerial(serial string) bool { + s := strings.TrimSpace(serial) + if s == "" { + return true + } + if _, ok := placeholderHardwareSerials[strings.ToLower(s)]; ok { + return true + } + // A serial that is the same character repeated (all zeros, all dots, all dashes, etc.) is never a real identity. + for i := 1; i < len(s); i++ { + if s[i] != s[0] { + return false + } + } + return true +} + // MDMEnrollStatus defines the possible MDM enrollment statuses. type MDMEnrollStatus string diff --git a/server/fleet/hosts_test.go b/server/fleet/hosts_test.go index 960d380a97e..69f3e9cf241 100644 --- a/server/fleet/hosts_test.go +++ b/server/fleet/hosts_test.go @@ -449,3 +449,62 @@ func TestMDMNameFromServerURL(t *testing.T) { }) } } + +func TestIsPlaceholderHardwareSerial(t *testing.T) { + for _, tc := range []struct { + name string + serial string + want bool + }{ + // Empty / whitespace. + {"empty", "", true}, + {"whitespace only", " ", true}, + {"tabs and newline", "\t \n", true}, + + // Known OEM/BIOS placeholders, matched case-insensitively and trimmed. + {"to be filled exact", "To Be Filled By O.E.M.", true}, + {"to be filled lower", "to be filled by o.e.m.", true}, + {"to be filled upper", "TO BE FILLED BY O.E.M.", true}, + {"to be filled padded", " To Be Filled By O.E.M. ", true}, + {"default string", "Default string", true}, + {"system serial number", "System Serial Number", true}, + {"not specified", "Not Specified", true}, + {"not applicable", "Not Applicable", true}, + {"none", "None", true}, + {"oem", "OEM", true}, + {"o.e.m.", "O.E.M.", true}, + {"default", "Default", true}, + {"unknown", "Unknown", true}, + {"chassis serial number", "Chassis Serial Number", true}, + {"base board serial number", "Base Board Serial Number", true}, + {"baseboard serial number", "Baseboard Serial Number", true}, + {"sequential 123456789", "123456789", true}, + {"sequential 0123456789", "0123456789", true}, + {"sequential 1234567890", "1234567890", true}, + {"sequential 1234567", "1234567", true}, + {"n/a", "N/A", true}, + {"na", "na", true}, + {"invalid", "INVALID", true}, + + // Repeated-character heuristic (cannot be enumerated). + {"single zero", "0", true}, + {"all zeros", "00000000", true}, + {"all x", "xxxxxxx", true}, + {"all dashes", "-------", true}, + {"all dots", "........", true}, + + // Real, unique serials must NOT be flagged. + {"dell service tag", "7XQ2W13", false}, + {"lenovo serial", "PF0ABCDE", false}, + {"apple-style serial", "C02ABCDEFGHJ", false}, + {"vmware unique", "VMware-56 4d 1a 2b 3c", false}, + {"none as substring", "NONE123", false}, + {"default as substring", "DEFAULT-7H2K", false}, + {"leading zeros but real", "00000001", false}, + {"long alphanumeric", "ABC123XYZ789", false}, + } { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, IsPlaceholderHardwareSerial(tc.serial)) + }) + } +} diff --git a/server/service/mdm_test.go b/server/service/mdm_test.go index 80f4ddb377d..7e04a3b2f49 100644 --- a/server/service/mdm_test.go +++ b/server/service/mdm_test.go @@ -3154,9 +3154,14 @@ func TestGetDeviceSoftwareMDMCommandResultsVPPMetadata(t *testing.T) { } // TestProcessIncomingMDMCmdsDevDetailLinkage exercises the OMA-DM DevDetail-based linkage path that closes the race -// where mdm_windows_enrollments.host_uuid is empty after Azure / automatic enrollment. See issue #45380. The primary -// linkage mechanism is implemented in processIncomingMDMCmds: when an enrollment row has empty host_uuid, the server -// (a) parses any incoming Results for ./DevDetail/Ext/Microsoft/SMBIOSSerialNumber and links the host, then +// where mdm_windows_enrollments.host_uuid stays empty after a Windows BYOD enrollment (Settings > Access work or +// school > Connect). BYOD is an Azure/automatic enrollment under the hood: the WSTEP RST carries only +// a UPN, so the row is inserted unlinked and host_uuid is backfilled later. The same root cause (and this fix) also +// covers Entra-join and Autopilot/OOBE enrollments; BYOD is the headline because fleetd is already running, so the host +// row exists and linkage resolves in one round-trip instead of waiting for the osquery cycle. +// +// The primary linkage mechanism is implemented in processIncomingMDMCmds: when an enrollment row has empty host_uuid, +// the server (a) parses any incoming Results for ./DevDetail/Ext/Microsoft/SMBIOSSerialNumber and links the host, then // (b) injects a Get for the same URI into the outgoing response so the device replies on the next round-trip. func TestProcessIncomingMDMCmdsDevDetailLinkage(t *testing.T) { const ( @@ -3164,6 +3169,8 @@ func TestProcessIncomingMDMCmdsDevDetailLinkage(t *testing.T) { testSerial = "ABC123XYZ" testHostUUID = "host-uuid-from-osquery" testHostID uint = 99 + // CmdRef value is never asserted; a constant keeps the generated SyncML deterministic. + resultsCmdRef = "results-cmdref" ) // buildReqMsg builds a minimal valid SyncML request. extraBody (if non-empty) is inserted into ; use it to @@ -3202,6 +3209,7 @@ func TestProcessIncomingMDMCmdsDevDetailLinkage(t *testing.T) { case *Service: svcImpl = v } + require.NotNil(t, svcImpl, "could not extract *Service from %T", svc) ds.MDMWindowsSaveResponseFunc = func(ctx context.Context, _ *fleet.MDMWindowsEnrolledDevice, _ fleet.EnrichedSyncML, _ []string) (*fleet.MDMWindowsSaveResponseResult, error) { return nil, nil } @@ -3224,6 +3232,43 @@ func TestProcessIncomingMDMCmdsDevDetailLinkage(t *testing.T) { return false } + // resultsBody builds a command (the device's reply to our DevDetail Get) with one per + // (LocURI, Data) pair. + resultsBody := func(items ...[2]string) string { + var b strings.Builder + b.WriteString("r11" + resultsCmdRef + "") + for _, it := range items { + b.WriteString(fmt.Sprintf("%s%s", it[0], it[1])) + } + b.WriteString("") + return b.String() + } + // serialResults is the common single-item reply carrying just the SMBIOS serial. + serialResults := func(serial string) string { + return resultsBody([2]string{devDetailSMBIOSSerialNumberURI, serial}) + } + + // stubLink wires the datastore funcs the linkage path calls when a host matches the reported serial. updated + // controls UpdateMDMWindowsEnrollmentsHostUUID's return: true models the first link; false models the row already + // holding this host_uuid (the WHERE host_uuid <> ? guard short-circuited). The enroll user is non-UPN so + // LinkWindowsHostMDMEnrollment skips the post-link UPN/SCIM bookkeeping (and, when updated==false, returns before + // MDMWindowsGetEnrolledDeviceWithDeviceID is consulted at all). + stubLink := func(t *testing.T, ds *mock.Store, updated bool) { + t.Helper() + ds.WindowsHostLiteByHardwareSerialFunc = func(_ context.Context, serial string) (*fleet.HostLite, error) { + assert.Equal(t, testSerial, serial) + return &fleet.HostLite{ID: testHostID, UUID: testHostUUID}, nil + } + ds.UpdateMDMWindowsEnrollmentsHostUUIDFunc = func(_ context.Context, hostUUID, mdmDeviceID string) (bool, error) { + assert.Equal(t, testHostUUID, hostUUID) + assert.Equal(t, testDeviceID, mdmDeviceID) + return updated, nil + } + ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFunc = func(_ context.Context, _ string) (*fleet.MDMWindowsEnrolledDevice, error) { + return &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, MDMEnrollUserID: ""}, nil + } + } + t.Run("unlinked enrollment: Get for DevDetail SMBIOSSerialNumber is injected", func(t *testing.T) { svc, ds, ctx := newSvc(t) enrolledDevice := &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, HostUUID: ""} @@ -3262,33 +3307,9 @@ func TestProcessIncomingMDMCmdsDevDetailLinkage(t *testing.T) { t.Run("Results with SMBIOS serial trigger linkage and skip the redundant Get", func(t *testing.T) { svc, ds, ctx := newSvc(t) enrolledDevice := &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, HostUUID: ""} + stubLink(t, ds, true) - ds.WindowsHostLiteByHardwareSerialFunc = func(_ context.Context, serial string) (*fleet.HostLite, error) { - assert.Equal(t, testSerial, serial) - return &fleet.HostLite{ID: testHostID, UUID: testHostUUID}, nil - } - ds.UpdateMDMWindowsEnrollmentsHostUUIDFunc = func(_ context.Context, hostUUID, mdmDeviceID string) (bool, error) { - assert.Equal(t, testHostUUID, hostUUID) - assert.Equal(t, testDeviceID, mdmDeviceID) - return true, nil - } - // Non-UPN enroll user means LinkWindowsHostMDMEnrollment skips the post-link UPN/SCIM bookkeeping. - ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFunc = func(_ context.Context, _ string) (*fleet.MDMWindowsEnrolledDevice, error) { - return &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, MDMEnrollUserID: ""}, nil - } - - extraBody := fmt.Sprintf(` - r1 - 1 - %s - - %s - %s - - `, uuid.NewString(), devDetailSMBIOSSerialNumberURI, testSerial) - reqMsg := buildReqMsg(t, extraBody) - - cmds, err := svc.processIncomingMDMCmds(ctx, enrolledDevice, reqMsg, RequestAuthStateTrusted) + cmds, err := svc.processIncomingMDMCmds(ctx, enrolledDevice, buildReqMsg(t, serialResults(testSerial)), RequestAuthStateTrusted) require.NoError(t, err) assert.True(t, ds.WindowsHostLiteByHardwareSerialFuncInvoked) assert.True(t, ds.UpdateMDMWindowsEnrollmentsHostUUIDFuncInvoked) @@ -3296,26 +3317,16 @@ func TestProcessIncomingMDMCmdsDevDetailLinkage(t *testing.T) { assert.False(t, hasGetForDevDetailSerial(cmds), "after successful linkage, no further Get should be injected") }) - t.Run("serial without matching host: Get is reinjected for retry", func(t *testing.T) { + t.Run("serial with no matching host (NotFound): Get is reinjected for retry", func(t *testing.T) { svc, ds, ctx := newSvc(t) enrolledDevice := &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, HostUUID: ""} - + // Production returns a fleet NotFound (not sql.ErrNoRows) when no Windows host matches the serial: the host + // hasn't enrolled in osquery yet, or the serial is ambiguous. This is the common, non-error retry path. ds.WindowsHostLiteByHardwareSerialFunc = func(_ context.Context, _ string) (*fleet.HostLite, error) { - return nil, sql.ErrNoRows + return nil, ¬FoundError{} } - extraBody := fmt.Sprintf(` - r1 - 1 - %s - - %s - %s - - `, uuid.NewString(), devDetailSMBIOSSerialNumberURI, testSerial) - reqMsg := buildReqMsg(t, extraBody) - - cmds, err := svc.processIncomingMDMCmds(ctx, enrolledDevice, reqMsg, RequestAuthStateTrusted) + cmds, err := svc.processIncomingMDMCmds(ctx, enrolledDevice, buildReqMsg(t, serialResults(testSerial)), RequestAuthStateTrusted) require.NoError(t, err) assert.True(t, ds.WindowsHostLiteByHardwareSerialFuncInvoked) assert.False(t, ds.UpdateMDMWindowsEnrollmentsHostUUIDFuncInvoked) @@ -3323,38 +3334,64 @@ func TestProcessIncomingMDMCmdsDevDetailLinkage(t *testing.T) { assert.True(t, hasGetForDevDetailSerial(cmds), "without a host match, the Get is reinjected for the next session") }) - t.Run("SMBIOSSerialNumber Item is not the first one: still found", func(t *testing.T) { + t.Run("serial lookup fails with an unexpected error: non-fatal, Get reinjected", func(t *testing.T) { svc, ds, ctx := newSvc(t) enrolledDevice := &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, HostUUID: ""} - - ds.WindowsHostLiteByHardwareSerialFunc = func(_ context.Context, serial string) (*fleet.HostLite, error) { - assert.Equal(t, testSerial, serial) - return &fleet.HostLite{ID: testHostID, UUID: testHostUUID}, nil - } - ds.UpdateMDMWindowsEnrollmentsHostUUIDFunc = func(_ context.Context, _, _ string) (bool, error) { - return true, nil + // A non-NotFound error (e.g. a real DB failure) is logged and handled, but linkage stays non-fatal: the + // management session must not fail and the Get must be reinjected so a later session retries. + ds.WindowsHostLiteByHardwareSerialFunc = func(_ context.Context, _ string) (*fleet.HostLite, error) { + return nil, errors.New("db unavailable") } - ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFunc = func(_ context.Context, _ string) (*fleet.MDMWindowsEnrolledDevice, error) { - return &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, MDMEnrollUserID: ""}, nil + + cmds, err := svc.processIncomingMDMCmds(ctx, enrolledDevice, buildReqMsg(t, serialResults(testSerial)), RequestAuthStateTrusted) + require.NoError(t, err, "a serial-lookup error must not fail the management session") + assert.True(t, ds.WindowsHostLiteByHardwareSerialFuncInvoked) + assert.False(t, ds.UpdateMDMWindowsEnrollmentsHostUUIDFuncInvoked) + assert.Empty(t, enrolledDevice.HostUUID) + assert.True(t, hasGetForDevDetailSerial(cmds), "after a lookup error, the Get is reinjected for the next session") + }) + + t.Run("Results carries the URI but an empty serial: no lookup, Get reinjected", func(t *testing.T) { + svc, ds, ctx := newSvc(t) + enrolledDevice := &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, HostUUID: ""} + + // Whitespace-only Data trims to empty, so there is no usable serial and no host lookup should happen. + cmds, err := svc.processIncomingMDMCmds(ctx, enrolledDevice, buildReqMsg(t, serialResults(" ")), RequestAuthStateTrusted) + require.NoError(t, err) + assert.False(t, ds.WindowsHostLiteByHardwareSerialFuncInvoked, "empty serial must not trigger a host lookup") + assert.Empty(t, enrolledDevice.HostUUID) + assert.True(t, hasGetForDevDetailSerial(cmds), "no usable serial means the Get is reinjected") + }) + + t.Run("placeholder serial: no lookup, no mislink, Get reinjected", func(t *testing.T) { + svc, ds, ctx := newSvc(t) + enrolledDevice := &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, HostUUID: ""} + // Reproduce the staggered-mislink trap: an unrelated host already carries this junk placeholder serial, so a + // naive lookup would return exactly one match and link THIS enrollment to the wrong host. The placeholder guard + // must stop the lookup from ever running; these devices link via the osquery MDMDeviceID backstop instead. + ds.WindowsHostLiteByHardwareSerialFunc = func(_ context.Context, _ string) (*fleet.HostLite, error) { + return &fleet.HostLite{ID: 12345, UUID: "some-other-hosts-uuid"}, nil } + cmds, err := svc.processIncomingMDMCmds(ctx, enrolledDevice, buildReqMsg(t, serialResults("To Be Filled By O.E.M.")), RequestAuthStateTrusted) + require.NoError(t, err) + assert.False(t, ds.WindowsHostLiteByHardwareSerialFuncInvoked, "placeholder serial must not trigger a host lookup") + assert.Empty(t, enrolledDevice.HostUUID, "must not link to an unrelated host that shares a placeholder serial") + assert.True(t, hasGetForDevDetailSerial(cmds), "placeholder serial defers linkage; the Get is reinjected") + }) + + t.Run("SMBIOSSerialNumber Item is not the first one: still found", func(t *testing.T) { + svc, ds, ctx := newSvc(t) + enrolledDevice := &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, HostUUID: ""} + stubLink(t, ds, true) + // The serial is the second Item in the Results command, not the first. A naive Items[0]-only scan misses it. - extraBody := fmt.Sprintf(` - r1 - 1 - %s - - ./DevDetail/Ext/Microsoft/DeviceName - DESKTOP-OTHER - - - %s - %s - - `, uuid.NewString(), devDetailSMBIOSSerialNumberURI, testSerial) - reqMsg := buildReqMsg(t, extraBody) + body := resultsBody( + [2]string{"./DevDetail/Ext/Microsoft/DeviceName", "DESKTOP-OTHER"}, + [2]string{devDetailSMBIOSSerialNumberURI, testSerial}, + ) - cmds, err := svc.processIncomingMDMCmds(ctx, enrolledDevice, reqMsg, RequestAuthStateTrusted) + cmds, err := svc.processIncomingMDMCmds(ctx, enrolledDevice, buildReqMsg(t, body), RequestAuthStateTrusted) require.NoError(t, err) assert.Equal(t, testHostUUID, enrolledDevice.HostUUID) assert.False(t, hasGetForDevDetailSerial(cmds)) @@ -3363,28 +3400,11 @@ func TestProcessIncomingMDMCmdsDevDetailLinkage(t *testing.T) { t.Run("link already applied by another path: HostUUID still refreshed in-memory", func(t *testing.T) { svc, ds, ctx := newSvc(t) enrolledDevice := &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, HostUUID: ""} - - ds.WindowsHostLiteByHardwareSerialFunc = func(_ context.Context, _ string) (*fleet.HostLite, error) { - return &fleet.HostLite{ID: testHostID, UUID: testHostUUID}, nil - } // updated=false simulates the row already holding host_uuid=testHostUUID (the WHERE host_uuid <> ? guard // short-circuited). In-memory state must still be refreshed so no redundant Get is sent. - ds.UpdateMDMWindowsEnrollmentsHostUUIDFunc = func(_ context.Context, _, _ string) (bool, error) { - return false, nil - } + stubLink(t, ds, false) - extraBody := fmt.Sprintf(` - r1 - 1 - %s - - %s - %s - - `, uuid.NewString(), devDetailSMBIOSSerialNumberURI, testSerial) - reqMsg := buildReqMsg(t, extraBody) - - cmds, err := svc.processIncomingMDMCmds(ctx, enrolledDevice, reqMsg, RequestAuthStateTrusted) + cmds, err := svc.processIncomingMDMCmds(ctx, enrolledDevice, buildReqMsg(t, serialResults(testSerial)), RequestAuthStateTrusted) require.NoError(t, err) assert.Equal(t, testHostUUID, enrolledDevice.HostUUID, "even updated=false must refresh in-memory HostUUID") assert.False(t, hasGetForDevDetailSerial(cmds), "in-memory HostUUID is now set; no redundant Get") diff --git a/server/service/microsoft_mdm.go b/server/service/microsoft_mdm.go index ade5115cd22..f942b438a4d 100644 --- a/server/service/microsoft_mdm.go +++ b/server/service/microsoft_mdm.go @@ -44,10 +44,7 @@ import ( const maxRequestLogSize = 10240 -// devDetailSMBIOSSerialNumberURI is the OMA-DM LocURI for the device's SMBIOS serial number. Used to link an unlinked -// Windows MDM enrollment (mdm_windows_enrollments.host_uuid = ”) to a Fleet host record without waiting for osquery's -// directIngestMDMDeviceIDWindows backstop. See LinkWindowsHostMDMEnrollment in server/service/osquery_utils for the -// linkage logic shared with the osquery path. +// devDetailSMBIOSSerialNumberURI is the OMA-DM LocURI for the device's SMBIOS serial number. const devDetailSMBIOSSerialNumberURI = "./DevDetail/Ext/Microsoft/SMBIOSSerialNumber" type SoapRequestContainer struct { @@ -1700,8 +1697,7 @@ func (svc *Service) processIncomingAlertsCommands(ctx context.Context, messageID // tryLinkUnlinkedEnrollmentFromDevDetail scans an incoming SyncML message for a Results command answering an earlier // Get for ./DevDetail/Ext/Microsoft/SMBIOSSerialNumber, and if found, looks up the host by hardware_serial and links -// the enrollment to it. Returns true if a linkage was established (the in-memory enrolledDevice.HostUUID is updated so -// downstream callers in the same request see the linked state). +// the enrollment to it. Returns true if a linkage was established. // // Any error is non-fatal: this is the primary linkage path but osquery direct-ingest still runs as a backstop, and the // Get is reinjected on every subsequent session until linkage succeeds. @@ -1725,34 +1721,40 @@ scan: if item.Data == nil { continue } - serial = strings.TrimSpace(item.Data.Content) - if serial != "" { - break scan + candidate := strings.TrimSpace(item.Data.Content) + // Skip empty or well-known placeholder serials (whitebox/consumer BIOS defaults, un-sysprepped VM + // templates). They are not unique identities, so linking on one risks attaching this enrollment to an + // unrelated host that happens to report the same junk value. Such devices fall back to the osquery + // directIngestMDMDeviceIDWindows backstop, which links by the unique MDMDeviceID instead. + if fleet.IsPlaceholderHardwareSerial(candidate) { + continue } + serial = candidate + break scan } } if serial == "" { return false } - host, err := svc.ds.WindowsHostLiteByHardwareSerial(ctx, serial) + // Require the primary DB: the hosts row may have been inserted seconds ago by osquery enroll, just before this + // first SyncML management session arrived. A replica-lag read would return a false NotFound and delay linkage to a + // later session, defeating the point of this path. + host, err := svc.ds.WindowsHostLiteByHardwareSerial(ctxdb.RequirePrimary(ctx, true), serial) if err != nil { if !fleet.IsNotFound(err) { - svc.logger.WarnContext(ctx, "windows mdm: host lookup by serial failed", - "err", err, "device_id", enrolledDevice.MDMDeviceID) + svc.logger.ErrorContext(ctx, "windows mdm: host lookup by serial failed", "err", err, "device_id", enrolledDevice.MDMDeviceID) + ctxerr.Handle(ctx, err) } - // NotFound means the host hasn't enrolled in osquery yet; we'll retry next session. + // NotFound means the host hasn't enrolled in osquery yet (hosts row not created yet); we'll retry next session. return false } updated, err := osquery_utils.LinkWindowsHostMDMEnrollment(ctx, svc.logger, svc.ds, host.ID, host.UUID, enrolledDevice.MDMDeviceID) if err != nil { - svc.logger.WarnContext(ctx, "windows mdm: link by DevDetail failed", - "err", err, "device_id", enrolledDevice.MDMDeviceID) + svc.logger.ErrorContext(ctx, "windows mdm: link by DevDetail failed", "err", err, "device_id", enrolledDevice.MDMDeviceID) + ctxerr.Handle(ctx, err) return false } - // Always refresh in-memory HostUUID after a successful link attempt, even when updated==false. updated=false means - // UpdateMDMWindowsEnrollmentsHostUUID's `WHERE host_uuid <> ?` guard short-circuited because the DB row already - // holds this host's UUID (e.g. another concurrent path linked it, or the in-memory row predates a writer-side - // update). Without this refresh the rest of the request would still see HostUUID="" and reinject another Get. + // Always refresh in-memory HostUUID after a successful link attempt. enrolledDevice.HostUUID = host.UUID return updated } @@ -1919,9 +1921,7 @@ func (svc *Service) processIncomingMDMCmds(ctx context.Context, enrolledDevice * // until linkage succeeds; osquery direct-ingest remains as a backstop for hosts that never reply to DevDetail. // // The Get uses a stable fleet-internal CmdID instead of a fresh UUID so that MDMWindowsSaveResponse can recognize - // and skip it when checking for "unmatched Windows MDM commands". The Get is never inserted into windows_mdm_commands - // (it's purely a protocol-level linkage probe), so without that filter the device's Status/Results reply would log - // a warning on every session for unlinked enrollments. + // and skip it when checking for "unmatched Windows MDM commands". if enrolledDevice.HostUUID == "" { get := newSyncMLCmdGet(devDetailSMBIOSSerialNumberURI) get.CmdID = mdm_types.CmdID{Value: fleet.FleetInternalCmdIDPrefix + "devdetail-smbios-serial"} @@ -3274,9 +3274,7 @@ func newSyncMLCmdNode(cmdVerb string, cmdTarget string) *mdm_types.SyncMLCmd { func newSyncMLCmdGet(cmdTarget string) *mdm_types.SyncMLCmd { verb := fleet.CmdGet item := newSyncMLItem(nil, &cmdTarget, nil, nil, nil) - cmd := newSyncMLCmdWithItem(&verb, nil, item) - cmd.CmdID = mdm_types.CmdID{Value: uuid.New().String()} - return cmd + return newSyncMLCmdWithItem(&verb, nil, item) } // newSyncMLCmdInt creates a new SyncML command with text data diff --git a/server/service/osquery_utils/queries.go b/server/service/osquery_utils/queries.go index 83f1fa5b2ee..17432d05bc7 100644 --- a/server/service/osquery_utils/queries.go +++ b/server/service/osquery_utils/queries.go @@ -3050,9 +3050,6 @@ func directIngestMDMDeviceIDWindows(ctx context.Context, logger *slog.Logger, ho // this same hostUUID, so the `WHERE host_uuid <> ?` guard short-circuited; (b) no row matched mdmDeviceID at all (e.g. // the enrollment was deleted concurrently). Callers that depend on linkage being applied should re-read the enrollment // rather than infer it from the boolean alone. -// -// This helper is shared by the osquery direct-ingest path (legacy) and the OMA-DM DevDetail SyncML path (primary), so -// both linkage triggers run the same post-link bookkeeping exactly once per linkage. func LinkWindowsHostMDMEnrollment(ctx context.Context, logger *slog.Logger, ds fleet.Datastore, hostID uint, hostUUID, mdmDeviceID string) (bool, error) { updated, err := ds.UpdateMDMWindowsEnrollmentsHostUUID(ctx, hostUUID, mdmDeviceID) if err != nil { From 36202ee2d9b5e373842f6f9287c664d0f2293d49 Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky <2685025+getvictor@users.noreply.github.com> Date: Thu, 28 May 2026 22:45:34 +0000 Subject: [PATCH 5/7] Code review fixes --- server/service/microsoft_mdm.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/server/service/microsoft_mdm.go b/server/service/microsoft_mdm.go index f942b438a4d..f2b3f41aff8 100644 --- a/server/service/microsoft_mdm.go +++ b/server/service/microsoft_mdm.go @@ -1723,9 +1723,8 @@ scan: } candidate := strings.TrimSpace(item.Data.Content) // Skip empty or well-known placeholder serials (whitebox/consumer BIOS defaults, un-sysprepped VM - // templates). They are not unique identities, so linking on one risks attaching this enrollment to an - // unrelated host that happens to report the same junk value. Such devices fall back to the osquery - // directIngestMDMDeviceIDWindows backstop, which links by the unique MDMDeviceID instead. + // templates). Such devices fall back to the osquery directIngestMDMDeviceIDWindows backstop, which links by + // the unique MDMDeviceID instead. if fleet.IsPlaceholderHardwareSerial(candidate) { continue } From 0f5a921259f86c5811d162b3fd5d3097c5178572 Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky <2685025+getvictor@users.noreply.github.com> Date: Fri, 29 May 2026 16:48:19 -0500 Subject: [PATCH 6/7] Konstantin's review comment --- server/service/osquery_utils/queries.go | 1 + 1 file changed, 1 insertion(+) diff --git a/server/service/osquery_utils/queries.go b/server/service/osquery_utils/queries.go index 17432d05bc7..61235bacda2 100644 --- a/server/service/osquery_utils/queries.go +++ b/server/service/osquery_utils/queries.go @@ -3065,6 +3065,7 @@ func LinkWindowsHostMDMEnrollment(ctx context.Context, logger *slog.Logger, ds f if device == nil || !microsoft_mdm.IsValidUPN(device.MDMEnrollUserID) { return updated, nil } + device.HostUUID = hostUUID // in case the read was stale due to replication lag // Update the host's MDM enrolled flags to show it as a manual enrollment so it doesn't take two full refreshes to // reflect this state. if device.MDMNotInOOBE { From 681083bb6093a811ed343d4a7a9ee93d17e0f295 Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky <2685025+getvictor@users.noreply.github.com> Date: Mon, 1 Jun 2026 18:23:29 -0500 Subject: [PATCH 7/7] Update changes/45380-windows-mdm-enrollment-row-linkage Co-authored-by: Konstantin Sykulev --- changes/45380-windows-mdm-enrollment-row-linkage | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes/45380-windows-mdm-enrollment-row-linkage b/changes/45380-windows-mdm-enrollment-row-linkage index c320054aa51..44afe710d3d 100644 --- a/changes/45380-windows-mdm-enrollment-row-linkage +++ b/changes/45380-windows-mdm-enrollment-row-linkage @@ -1 +1 @@ -- Fixed a race after Windows BYOD MDM enrollment (Settings > Access work or school > Connect) where `mdm_windows_enrollments.host_uuid` stayed empty for several seconds, causing server-side enrollment lookups to miss. The enrollment is now linked to the Fleet host record at the first management session via OMA-DM DevDetail/SMBIOSSerialNumber instead of waiting for osquery's distributed-read backfill. +- Fixed a race condition after Windows BYOD MDM enrollment (Settings > Access work or school > Connect) where `mdm_windows_enrollments.host_uuid` stayed empty for several seconds, causing server-side enrollment lookups to miss. The enrollment is now linked to the Fleet host record at the first management session via OMA-DM DevDetail/SMBIOSSerialNumber instead of waiting for osquery's distributed-read backfill.