diff --git a/changes/48760-windows-mdm-enrollment-state b/changes/48760-windows-mdm-enrollment-state new file mode 100644 index 00000000000..aa6239b79cd --- /dev/null +++ b/changes/48760-windows-mdm-enrollment-state @@ -0,0 +1 @@ +- Fixed a bug where fleetd could not start on-demand Windows MDM sessions on some Windows hosts, leaving queued Windows MDM commands pending for up to 8 hours. fleetd now recognizes the Fleet enrollment by any enrolled (non-zero) `EnrollmentState`. diff --git a/orbit/pkg/update/execwinapi.go b/orbit/pkg/update/execwinapi.go index 84d70e351f1..60cde94211d 100644 --- a/orbit/pkg/update/execwinapi.go +++ b/orbit/pkg/update/execwinapi.go @@ -1,5 +1,11 @@ package update +import ( + "regexp" + + "github.com/fleetdm/fleet/v4/server/mdm/microsoft/syncml" +) + // Exported so that it can be used in tools/ (so that it can be built for // Windows and tested on a Windows machine). Otherwise not meant to be used // from outside this package. @@ -8,3 +14,18 @@ type WindowsMDMEnrollmentArgs struct { HostUUID string OrbitNodeKey string } + +// windowsEnrollmentStateUnknown is the EnrollmentState value that means "unknown / not enrolled". +const windowsEnrollmentStateUnknown = 0 + +// windowsEnrollmentGUIDRe matches a standard enrollment GUID (8-4-4-4-12 hex). +var windowsEnrollmentGUIDRe = regexp.MustCompile(`^[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}$`) + +// isActiveFleetEnrollment reports whether an HKLM\SOFTWARE\Microsoft\Enrollments\ entry is Fleet's active Windows MDM +// enrollment. It matches when the ProviderID is Fleet's, the EnrollmentState is a non-zero (enrolled) value, and the subkey name is a +// well-formed enrollment GUID. +func isActiveFleetEnrollment(providerID string, state uint64, subkeyName string) bool { + return providerID == syncml.DocProvisioningAppProviderID && + state != windowsEnrollmentStateUnknown && + windowsEnrollmentGUIDRe.MatchString(subkeyName) +} diff --git a/orbit/pkg/update/execwinapi_test.go b/orbit/pkg/update/execwinapi_test.go new file mode 100644 index 00000000000..d0d61bd7052 --- /dev/null +++ b/orbit/pkg/update/execwinapi_test.go @@ -0,0 +1,32 @@ +package update + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIsActiveFleetEnrollment(t *testing.T) { + const fleetGUID = "39771ECF-778A-41BD-AD7A-C6DA11E20FC8" + + testCases := []struct { + name string + providerID string + state uint64 + subkeyName string + want bool + }{ + {name: "enrolled state 1", providerID: "Fleet", state: 1, subkeyName: fleetGUID, want: true}, + // #48760: the previous code pinned to state == 1 and rejected 3, the value seen on affected devices, so on-demand syncs failed. + {name: "enrolled state 3", providerID: "Fleet", state: 3, subkeyName: fleetGUID, want: true}, + {name: "state 0 rejected", providerID: "Fleet", state: 0, subkeyName: fleetGUID, want: false}, + {name: "non-fleet provider rejected", providerID: "MS DM Server", state: 3, subkeyName: fleetGUID, want: false}, + {name: "malformed subkey name rejected", providerID: "Fleet", state: 3, subkeyName: "not-a-guid", want: false}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, isActiveFleetEnrollment(tc.providerID, tc.state, tc.subkeyName)) + }) + } +} diff --git a/orbit/pkg/update/execwinapi_windows.go b/orbit/pkg/update/execwinapi_windows.go index 20a7fd9ffae..edf705ef4bd 100644 --- a/orbit/pkg/update/execwinapi_windows.go +++ b/orbit/pkg/update/execwinapi_windows.go @@ -11,14 +11,12 @@ import ( "net/http" "os/exec" "path/filepath" - "regexp" "strings" "syscall" "time" "unsafe" "github.com/fleetdm/fleet/v4/server/fleet" - "github.com/fleetdm/fleet/v4/server/mdm/microsoft/syncml" "github.com/rs/zerolog/log" "golang.org/x/sys/windows" "golang.org/x/sys/windows/registry" @@ -244,13 +242,9 @@ func TriggerWindowsMDMSync() error { return nil } -// windowsEnrollmentGUIDRe matches a standard enrollment GUID (8-4-4-4-12 hex). The matched subkey name becomes an argument to deviceenroller -// while orbit runs as SYSTEM, so we validate its shape before using it, even though writing the Enrollments key already requires admin. -var windowsEnrollmentGUIDRe = regexp.MustCompile(`^[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}$`) - // fleetMDMEnrollmentGUID returns the enrollment GUID of the active Fleet Windows MDM enrollment by scanning -// HKLM\SOFTWARE\Microsoft\Enrollments for the subkey whose ProviderID is Fleet's and whose EnrollmentState is active. The subkey name is -// the enrollment GUID that deviceenroller's /o argument expects. +// HKLM\SOFTWARE\Microsoft\Enrollments for the subkey whose ProviderID is Fleet's and whose EnrollmentState is active (see +// isActiveFleetEnrollment). The subkey name is the enrollment GUID that deviceenroller's /o argument expects. func fleetMDMEnrollmentGUID() (string, error) { const enrollmentsPath = `SOFTWARE\Microsoft\Enrollments` root, err := registry.OpenKey(registry.LOCAL_MACHINE, enrollmentsPath, registry.READ) @@ -264,10 +258,6 @@ func fleetMDMEnrollmentGUID() (string, error) { return "", fmt.Errorf("read enrollment subkeys: %w", err) } - // EnrollmentState == 1 is the active state observed for Fleet's MDM enrollment on tested Windows builds; the registry DWORD under - // Enrollments is not authoritatively documented by Microsoft. The ProviderID == "Fleet" check in the loop below scopes the match to - // Fleet's own enrollment, so this never selects an unrelated (e.g. Intune) enrollment that might use a different state value. - const enrollmentStateActive = 1 for _, name := range names { k, err := registry.OpenKey(registry.LOCAL_MACHINE, enrollmentsPath+`\`+name, registry.QUERY_VALUE) if err != nil { @@ -276,11 +266,8 @@ func fleetMDMEnrollmentGUID() (string, error) { providerID, _, providerErr := k.GetStringValue("ProviderID") state, _, stateErr := k.GetIntegerValue("EnrollmentState") k.Close() - if providerErr == nil && stateErr == nil && providerID == syncml.DocProvisioningAppProviderID && state == enrollmentStateActive { - // Don't hand a malformed subkey name to deviceenroller; skip it and keep looking for a well-formed enrollment GUID. - if !windowsEnrollmentGUIDRe.MatchString(name) { - continue - } + // A malformed subkey name (not a valid GUID) makes isActiveFleetEnrollment return false, so we keep scanning for a well-formed one. + if providerErr == nil && stateErr == nil && isActiveFleetEnrollment(providerID, state, name) { return name, nil } }