diff --git a/orbit/changes/12068-migration-sanity-check b/orbit/changes/12068-migration-sanity-check new file mode 100644 index 0000000000..07cfc6ba38 --- /dev/null +++ b/orbit/changes/12068-migration-sanity-check @@ -0,0 +1 @@ +* Ensure MDM migration modal is not shown, and enrollment commands are not run if the host is already enrolled into Fleet diff --git a/orbit/cmd/desktop/desktop.go b/orbit/cmd/desktop/desktop.go index 232fabc4d7..a6755e11da 100644 --- a/orbit/cmd/desktop/desktop.go +++ b/orbit/cmd/desktop/desktop.go @@ -237,6 +237,7 @@ func main() { ) mdmMigrator = useraction.NewMDMMigrator( swiftDialogPath, + fleetURL, 15*time.Minute, &mdmMigrationHandler{ client: client, diff --git a/orbit/cmd/orbit/orbit.go b/orbit/cmd/orbit/orbit.go index 2080380716..b82272ec1a 100644 --- a/orbit/cmd/orbit/orbit.go +++ b/orbit/cmd/orbit/orbit.go @@ -618,7 +618,7 @@ func main() { renewEnrollmentProfileCommandFrequency = time.Hour windowsMDMEnrollmentCommandFrequency = time.Hour ) - configFetcher := update.ApplyRenewEnrollmentProfileConfigFetcherMiddleware(orbitClient, renewEnrollmentProfileCommandFrequency) + configFetcher := update.ApplyRenewEnrollmentProfileConfigFetcherMiddleware(orbitClient, renewEnrollmentProfileCommandFrequency, fleetURL) switch runtime.GOOS { case "darwin": diff --git a/orbit/pkg/profiles/profiles_darwin.go b/orbit/pkg/profiles/profiles_darwin.go index feba9230d7..8d591bdcec 100644 --- a/orbit/pkg/profiles/profiles_darwin.go +++ b/orbit/pkg/profiles/profiles_darwin.go @@ -6,6 +6,7 @@ import ( "bytes" "encoding/json" "fmt" + "net/url" "os/exec" "github.com/fleetdm/fleet/v4/server/fleet" @@ -52,3 +53,54 @@ var execScript = func(script string) (*bytes.Buffer, error) { } return &outBuf, nil } + +// IsEnrolledIntoMatchingURL runs the `profiles` command to get the current MDM +// enrollment information and reports if the hostname of the MDM server +// supervising the device matches the hostname of the provided URL. +func IsEnrolledIntoMatchingURL(serverURL string) (bool, error) { + out, err := getMDMInfoFromProfilesCmd() + if err != nil { + return false, fmt.Errorf("calling /usr/bin/profiles: %w", err) + } + + // The output of the command is in the form: + // + // ``` + // Enrolled via DEP: No + // MDM enrollment: Yes (User Approved) + // MDM server: https://test.example.com/mdm/apple/mdm + // ``` + // + // If the host is not enrolled into an MDM, the last line is ommitted, + // so we need to check that: + // + // 1. We've got three rows + // 2. The last row matches our server URL + lines := bytes.Split(bytes.TrimSpace(out), []byte("\n")) + if len(lines) < 3 { + return false, nil + } + + parts := bytes.SplitN(lines[2], []byte(":"), 2) + if len(parts) < 2 { + return false, fmt.Errorf("splitting profiles output to get MDM server URL: %w", err) + } + + u, err := url.Parse(string(bytes.TrimSpace(parts[1]))) + if err != nil { + return false, fmt.Errorf("parsing URL from profiles command: %w", err) + } + + fu, err := url.Parse(serverURL) + if err != nil { + return false, fmt.Errorf("parsing provided Fleet URL: %w", err) + } + + return u.Hostname() == fu.Hostname(), nil +} + +// getMDMInfoFromProfilesCmd is declared as a variable so it can be overwritten by tests. +var getMDMInfoFromProfilesCmd = func() ([]byte, error) { + cmd := exec.Command("/usr/bin/profiles", "status", "-type", "enrollment") + return cmd.Output() +} diff --git a/orbit/pkg/profiles/profiles_darwin_test.go b/orbit/pkg/profiles/profiles_darwin_test.go index da53214106..cfdaf315ce 100644 --- a/orbit/pkg/profiles/profiles_darwin_test.go +++ b/orbit/pkg/profiles/profiles_darwin_test.go @@ -69,3 +69,73 @@ func TestGetFleetdConfig(t *testing.T) { } } + +func TestIsEnrolledIntoMatchingURL(t *testing.T) { + fleetURL := "https://valid.com" + cases := []struct { + cmdOut *string + cmdErr error + wantOut bool + wantErr bool + }{ + {nil, errors.New("test error"), false, true}, + {ptr.String(""), nil, false, false}, + {ptr.String(` +Enrolled via DEP: No +MDM enrollment: No + `), nil, false, false}, + { + ptr.String(` +Enrolled via DEP: Yes +MDM enrollment: Yes +MDM server: https://test.example.com + `), + nil, + false, + false, + }, + { + ptr.String(` +Enrolled via DEP: Yes +MDM enrollment: Yes +MDM server / https://test.example.com + `), + nil, + false, + false, + }, + { + ptr.String(` +Enrolled via DEP: Yes +MDM enrollment: Yes +MDM server: https://valid.com/mdm/apple/mdm + `), + nil, + true, + false, + }, + } + + origCmd := getMDMInfoFromProfilesCmd + t.Cleanup(func() { getMDMInfoFromProfilesCmd = origCmd }) + for _, c := range cases { + getMDMInfoFromProfilesCmd = func() ([]byte, error) { + if c.cmdOut == nil { + return nil, c.cmdErr + } + + var buf bytes.Buffer + buf.WriteString(*c.cmdOut) + return []byte(*c.cmdOut), nil + } + + out, err := IsEnrolledIntoMatchingURL(fleetURL) + if c.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + require.Equal(t, c.wantOut, out) + } + +} diff --git a/orbit/pkg/profiles/profiles_notdarwin.go b/orbit/pkg/profiles/profiles_notdarwin.go index 359a726e0a..39f5758ea0 100644 --- a/orbit/pkg/profiles/profiles_notdarwin.go +++ b/orbit/pkg/profiles/profiles_notdarwin.go @@ -7,3 +7,7 @@ import "github.com/fleetdm/fleet/v4/server/fleet" func GetFleetdConfig() (*fleet.MDMAppleFleetdConfig, error) { return nil, ErrNotImplemented } + +func IsEnrolledIntoMatchingURL(u string) (bool, error) { + return false, ErrNotImplemented +} diff --git a/orbit/pkg/profiles/profiles_notdarwin_test.go b/orbit/pkg/profiles/profiles_notdarwin_test.go index e49ef60549..55c4297751 100644 --- a/orbit/pkg/profiles/profiles_notdarwin_test.go +++ b/orbit/pkg/profiles/profiles_notdarwin_test.go @@ -13,3 +13,9 @@ func TestGetFleetdConfig(t *testing.T) { require.ErrorIs(t, ErrNotImplemented, err) require.Nil(t, config) } + +func TestIsEnrolledIntoMatchingURL(t *testing.T) { + enrolled, err := IsEnrolledIntoMatchingURL("https://test.example.com") + require.ErrorIs(t, ErrNotImplemented, err) + require.False(t, enrolled) +} diff --git a/orbit/pkg/update/notifications.go b/orbit/pkg/update/notifications.go index bad5526559..b9303e43cc 100644 --- a/orbit/pkg/update/notifications.go +++ b/orbit/pkg/update/notifications.go @@ -5,12 +5,15 @@ import ( "sync" "time" + "github.com/fleetdm/fleet/v4/orbit/pkg/profiles" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/rs/zerolog/log" ) type runCmdFunc func() error +type checkEnrollmentFunc func(url string) (bool, error) + // renewEnrollmentProfileConfigFetcher is a kind of middleware that wraps an // OrbitConfigFetcher and detects if the fleet server sent a notification to // renew the enrollment profile. If so, it runs the command (as root) to @@ -31,13 +34,19 @@ type renewEnrollmentProfileConfigFetcher struct { // runRenewEnrollmentProfile. runCmdFn runCmdFunc + // for tests, to be able to mock the function that checks for Fleet + // enrollment + checkEnrollmentFn checkEnrollmentFunc + // ensures only one command runs at a time, protects access to lastRun cmdMu sync.Mutex lastRun time.Time + + fleetURL string } -func ApplyRenewEnrollmentProfileConfigFetcherMiddleware(fetcher OrbitConfigFetcher, frequency time.Duration) OrbitConfigFetcher { - return &renewEnrollmentProfileConfigFetcher{Fetcher: fetcher, Frequency: frequency} +func ApplyRenewEnrollmentProfileConfigFetcherMiddleware(fetcher OrbitConfigFetcher, frequency time.Duration, fleetURL string) OrbitConfigFetcher { + return &renewEnrollmentProfileConfigFetcher{Fetcher: fetcher, Frequency: frequency, fleetURL: fleetURL} } // GetConfig calls the wrapped Fetcher's GetConfig method, and if the fleet @@ -46,17 +55,6 @@ func ApplyRenewEnrollmentProfileConfigFetcherMiddleware(fetcher OrbitConfigFetch func (h *renewEnrollmentProfileConfigFetcher) GetConfig() (*fleet.OrbitConfig, error) { cfg, err := h.Fetcher.GetConfig() - // TODO: download and use swiftDialog following the same patterns we - // use for Nudge. - // - // updaterHasTarget := h.UpdateRunner.HasRunnerOptTarget("swiftDialog") - // runnerHasLocalHash := h.UpdateRunner.HasLocalHash("swiftDialog") - // if !updaterHasTarget || !runnerHasLocalHash { - // log.Info().Msg("refreshing the update runner config with swiftDialog targets and hashes") - // log.Debug().Msgf("updater has target: %t, runner has local hash: %t", updaterHasTarget, runnerHasLocalHash) - // return cfg, h.setTargetsAndHashes() - // } - if err == nil && cfg.Notifications.RenewEnrollmentProfile { if h.cmdMu.TryLock() { defer h.cmdMu.Unlock() @@ -67,6 +65,24 @@ func (h *renewEnrollmentProfileConfigFetcher) GetConfig() (*fleet.OrbitConfig, e // updated mdm enrollment). // See https://github.com/fleetdm/fleet/pull/9409#discussion_r1084382455 if time.Since(h.lastRun) > h.Frequency { + // we perform this check locally on the client too to avoid showing the + // dialog if the client has already migrated but the Fleet server + // doesn't know about this state yet. + enrollFn := h.checkEnrollmentFn + if enrollFn == nil { + enrollFn = profiles.IsEnrolledIntoMatchingURL + } + enrolled, err := enrollFn(h.fleetURL) + if err != nil { + log.Error().Err(err).Msg("fetching enrollment status") + return cfg, nil + } + if enrolled { + log.Info().Msg("a request to renew the enrollment profile was processed but not executed because the host is already enrolled into Fleet.") + h.lastRun = time.Now() + return cfg, nil + } + fn := h.runCmdFn if fn == nil { fn = runRenewEnrollmentProfile diff --git a/orbit/pkg/update/notifications_test.go b/orbit/pkg/update/notifications_test.go index 784f5094ea..d0f9cf1b35 100644 --- a/orbit/pkg/update/notifications_test.go +++ b/orbit/pkg/update/notifications_test.go @@ -48,6 +48,9 @@ func TestRenewEnrollmentProfile(t *testing.T) { cmdGotCalled = true return c.cmdErr }, + checkEnrollmentFn: func(url string) (bool, error) { + return false, nil + }, } cfg, err := renewFetcher.GetConfig() @@ -72,15 +75,19 @@ func TestRenewEnrollmentProfilePrevented(t *testing.T) { } var cmdCallCount int + isEnrolled := false chProceed := make(chan struct{}) renewFetcher := &renewEnrollmentProfileConfigFetcher{ Fetcher: fetcher, Frequency: 2 * time.Second, // just to be safe with slow environments (CI) runCmdFn: func() error { - <-chProceed // will be unblocked only when allowed cmdCallCount++ // no need for sync, single-threaded call of this func is guaranteed by the fetcher's mutex return nil }, + checkEnrollmentFn: func(url string) (bool, error) { + <-chProceed // will be unblocked only when allowed + return isEnrolled, nil + }, } assertResult := func(cfg *fleet.OrbitConfig, err error) { @@ -120,6 +127,15 @@ func TestRenewEnrollmentProfilePrevented(t *testing.T) { cfg, err = renewFetcher.GetConfig() assertResult(cfg, err) + // wait for the fetcher's frequency to pass + time.Sleep(renewFetcher.Frequency) + + // this call doesn't execute the command since the host is already + // enrolled + isEnrolled = true + cfg, err = renewFetcher.GetConfig() + assertResult(cfg, err) + require.Equal(t, 2, cmdCallCount) // the initial call and the one after sleep } diff --git a/orbit/pkg/useraction/mdm_migration_darwin.go b/orbit/pkg/useraction/mdm_migration_darwin.go index d9739acb25..2a5f36b580 100644 --- a/orbit/pkg/useraction/mdm_migration_darwin.go +++ b/orbit/pkg/useraction/mdm_migration_darwin.go @@ -12,6 +12,7 @@ import ( "text/template" "time" + "github.com/fleetdm/fleet/v4/orbit/pkg/profiles" "github.com/rs/zerolog/log" ) @@ -46,19 +47,31 @@ Please contact your IT admin [here]({{ .ContactURL }}). // swiftDialog. type baseDialog struct { path string + fleetURL string interruptCh chan struct{} } -func newBaseDialog(path string) *baseDialog { - return &baseDialog{path: path, interruptCh: make(chan struct{})} +func newBaseDialog(path, fleetURL string) *baseDialog { + return &baseDialog{path: path, fleetURL: fleetURL, interruptCh: make(chan struct{})} } func (b *baseDialog) CanRun() bool { + // check if swiftDialog has been downloaded if _, err := os.Stat(b.path); err != nil { return false } - return true + // we perform this check locally on the client too to avoid showing the + // dialog if the client has already migrated but the Fleet server + // doesn't know about this state yet. + enrolled, err := profiles.IsEnrolledIntoMatchingURL(b.fleetURL) + if err != nil { + log.Error().Err(err).Msg("fetching enrollment status to show swiftDialog") + return false + } + + // only run the dialog if the host is not enrolled into Fleet + return !enrolled } // Exit sends the interrupt signal to try and stop the current swiftDialog @@ -126,10 +139,10 @@ func (b *baseDialog) render(flags ...string) (chan swiftDialogExitCode, chan err return exitCodeCh, errCh } -func NewMDMMigrator(path string, frequency time.Duration, handler MDMMigratorHandler) MDMMigrator { +func NewMDMMigrator(path, fleetURL string, frequency time.Duration, handler MDMMigratorHandler) MDMMigrator { return &swiftDialogMDMMigrator{ handler: handler, - baseDialog: newBaseDialog(path), + baseDialog: newBaseDialog(path, fleetURL), frequency: frequency, } } diff --git a/orbit/pkg/useraction/mdm_migration_notdarwin.go b/orbit/pkg/useraction/mdm_migration_notdarwin.go index 98615a193c..27de1f2c84 100644 --- a/orbit/pkg/useraction/mdm_migration_notdarwin.go +++ b/orbit/pkg/useraction/mdm_migration_notdarwin.go @@ -4,7 +4,7 @@ package useraction import "time" -func NewMDMMigrator(path string, frequency time.Duration, handler MDMMigratorHandler) MDMMigrator { +func NewMDMMigrator(path, fleetURL string, frequency time.Duration, handler MDMMigratorHandler) MDMMigrator { return &NoopMDMMigrator{} }