Fixed a bug where fleetd could not start on-demand Windows MDM session

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`.

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\<subkeyName> 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)
+}

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))
+		})
+	}
+}

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
 		}
 	}