From 2029cc98d9e2f1f548d5567deafcea7485bc12f4 Mon Sep 17 00:00:00 2001 From: Jordan Montgomery Date: Mon, 19 May 2025 13:23:05 -0400 Subject: [PATCH] Revert "Always install Escrowbuddy and Swift Dialog (#28742)" (#29264) This reverts commit a5bd50716dba38e17bee6b91eb362c296264da46 which was this PR: https://github.com/fleetdm/fleet/pull/28742 It was determined that the behavior changes here conflict with other changes being asked for by `customer-starchik`. Design to review and come up with a different strategy for improving the behavior this change originally was intended to fix - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) - For Orbit and Fleet Desktop changes: - [x] Make sure fleetd is compatible with the latest released version of Fleet (see [Must rule](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/workflows/fleetd-development-and-release-strategy.md)). - [x] Orbit runs on macOS, Linux and Windows. Check if the orbit feature/bugfix should only apply to one platform (`runtime.GOOS`). - [x] Manual QA must be performed in the three main OSs, macOS, Windows and Linux. - [x] Auto-update manual QA, from released version of component to new version (see [tools/tuf/test](../tools/tuf/test/README.md)). - [x] For unreleased bug fixes in a release candidate, confirmed that the fix is not expected to adversely impact load test results or alerted the release DRI if additional load testing is needed. --- orbit/CHANGELOG.md | 2 -- orbit/pkg/update/escrow_buddy.go | 19 ++++++++--------- orbit/pkg/update/escrow_buddy_test.go | 7 ++++--- orbit/pkg/update/swift_dialog.go | 30 +++++++++++---------------- tools/tuf/test/README.md | 3 --- tools/tuf/test/docker_package.sh | 3 --- 6 files changed, 25 insertions(+), 39 deletions(-) diff --git a/orbit/CHANGELOG.md b/orbit/CHANGELOG.md index 402a0f770f..fcfd7d1ee9 100644 --- a/orbit/CHANGELOG.md +++ b/orbit/CHANGELOG.md @@ -11,8 +11,6 @@ * Improved support for wide aspect ratio icons in the MDM setup experience and migration dialogs for Apple devices. -* Changed orbit to unconditionally install Escrow Buddy and Swift Dialog on macOS hosts. - * When fleetd on a Windows host installs an update it detects from TUF, also update the corresponding `DisplayVersion` in the Registry. * Added automatic extraction of .tar.gz/.tgz archives prior to running the associated install script. diff --git a/orbit/pkg/update/escrow_buddy.go b/orbit/pkg/update/escrow_buddy.go index 65f17fbfbc..b9579eb008 100644 --- a/orbit/pkg/update/escrow_buddy.go +++ b/orbit/pkg/update/escrow_buddy.go @@ -64,17 +64,7 @@ func (e *EscrowBuddyRunner) Run(cfg *fleet.OrbitConfig) error { return nil } - // For #25928 we are going to always install escrowBuddy as a target updaterHasTarget := e.updateRunner.HasRunnerOptTarget("escrowBuddy") - runnerHasLocalHash := e.updateRunner.HasLocalHash("escrowBuddy") - if !updaterHasTarget || !runnerHasLocalHash { - log.Info().Msg("refreshing the update runner config with Escrow Buddy targets and hashes") - log.Debug().Msgf("updater has target: %t, runner has local hash: %t", updaterHasTarget, runnerHasLocalHash) - if err := e.setTargetsAndHashes(); err != nil { - return fmt.Errorf("setting Escrow Buddy targets and hashes: %w", err) - } - } - // if the notification is false, it could mean that we shouldn't do // anything at all (eg: MDM is not configured) or that this host // doesn't need to rotate the key. @@ -93,6 +83,15 @@ func (e *EscrowBuddyRunner) Run(cfg *fleet.OrbitConfig) error { return nil } + runnerHasLocalHash := e.updateRunner.HasLocalHash("escrowBuddy") + if !updaterHasTarget || !runnerHasLocalHash { + log.Info().Msg("refreshing the update runner config with Escrow Buddy targets and hashes") + log.Debug().Msgf("updater has target: %t, runner has local hash: %t", updaterHasTarget, runnerHasLocalHash) + if err := e.setTargetsAndHashes(); err != nil { + return fmt.Errorf("setting Escrow Buddy targets and hashes: %w", err) + } + } + // Some macOS updates and upgrades reset the authorization database to its default state // which will deactivate Escrow Buddy and prevent FileVault key generation upon next login. log.Debug().Msg("EscrowBuddyRunner: re-enable Escrow Buddy in the authorization database") diff --git a/orbit/pkg/update/escrow_buddy_test.go b/orbit/pkg/update/escrow_buddy_test.go index 0772b9ec60..ccd3093834 100644 --- a/orbit/pkg/update/escrow_buddy_test.go +++ b/orbit/pkg/update/escrow_buddy_test.go @@ -47,14 +47,14 @@ func (s *escrowBuddyTestSuite) TestEscrowBuddyRotatesKey() { return nil } - // there's an error when the remote repo doesn't have the target yet even though config is not set + // no new target added if the notification is not set err := r.Run(cfg) - require.ErrorContains(t, err, "tuf: file not found") + require.NoError(t, err) targets := runner.updater.opt.Targets require.Len(t, targets, 0) require.Empty(t, cmdCalls) - // there's an error when the remote repo doesn't have the target yet and the config is set + // there's an error when the remote repo doesn't have the target yet cfg.Notifications.RotateDiskEncryptionKey = true err = r.Run(cfg) require.ErrorContains(t, err, "tuf: file not found") @@ -86,4 +86,5 @@ func (s *escrowBuddyTestSuite) TestEscrowBuddyRotatesKey() { require.Len(t, cmdCalls, 1) require.Equal(t, cmdCalls[0]["cmd"], "sh") require.Equal(t, cmdCalls[0]["args"], []string{"-c", "defaults write /Library/Preferences/com.netflix.Escrow-Buddy.plist GenerateNewKey -bool false"}) + } diff --git a/orbit/pkg/update/swift_dialog.go b/orbit/pkg/update/swift_dialog.go index b6d0866cc2..306ab0872f 100644 --- a/orbit/pkg/update/swift_dialog.go +++ b/orbit/pkg/update/swift_dialog.go @@ -1,16 +1,12 @@ package update import ( - "sync" - "github.com/fleetdm/fleet/v4/server/fleet" "github.com/rs/zerolog/log" ) type SwiftDialogDownloader struct { - UpdateRunner *Runner - triggeredSetupExperienceUpdate bool - runMu sync.Mutex + UpdateRunner *Runner } type SwiftDialogDownloaderOptions struct { @@ -38,13 +34,13 @@ func (s *SwiftDialogDownloader) Run(cfg *fleet.OrbitConfig) error { return nil } - if !s.runMu.TryLock() { - log.Debug().Msg("SwiftDialogDownloader: a previous instance is currently running, returning early") + // TODO: we probably want to ensure that swiftDialog is always installed if we're going to be + // using it offline. + if !cfg.Notifications.NeedsMDMMigration && !cfg.Notifications.RenewEnrollmentProfile && !cfg.Notifications.RunSetupExperience { + log.Debug().Msg("skipping swiftDialog update") return nil } - defer s.runMu.Unlock() - // For #25928 we are going to always install swiftDialog as a target updaterHasTarget := s.UpdateRunner.HasRunnerOptTarget("swiftDialog") runnerHasLocalHash := s.UpdateRunner.HasLocalHash("swiftDialog") if !updaterHasTarget || !runnerHasLocalHash { @@ -60,16 +56,14 @@ func (s *SwiftDialogDownloader) Run(cfg *fleet.OrbitConfig) error { s.UpdateRunner.updater.RemoveTargetInfo("swiftDialog") return err } - } - // If we're running setup experience and we have the hashes we need to make sure we trigger - // an immediate update to get SwiftDialog installed and usable. - if cfg.Notifications.RunSetupExperience && !s.triggeredSetupExperienceUpdate { - s.triggeredSetupExperienceUpdate = true - log.Debug().Msg("SwiftDialogDownloader: triggering update to install swiftDialog immediately during setup experience") - _, err := s.UpdateRunner.UpdateAction() - if err != nil { - return err + if cfg.Notifications.RunSetupExperience { + // Then update immediately, since we need to get swiftDialog quickly to show the setup + // experience + _, err := s.UpdateRunner.UpdateAction() + if err != nil { + return err + } } } diff --git a/tools/tuf/test/README.md b/tools/tuf/test/README.md index f15fd68188..75629a6e8b 100644 --- a/tools/tuf/test/README.md +++ b/tools/tuf/test/README.md @@ -38,9 +38,6 @@ GENERATE_MSI=1 \ GENERATE_MSI_ARM64=1 \ ENROLL_SECRET=6/EzU/+jPkxfTamWnRv1+IJsO4T9Etju \ FLEET_DESKTOP=1 \ -NUDGE=1 \ -SWIFT_DIALOG=1 \ -ESCROW_BUDDY=1 \ USE_FLEET_SERVER_CERTIFICATE=1 \ DEBUG=1 \ ./tools/tuf/test/main.sh diff --git a/tools/tuf/test/docker_package.sh b/tools/tuf/test/docker_package.sh index 6957f24d21..35b00c60cf 100755 --- a/tools/tuf/test/docker_package.sh +++ b/tools/tuf/test/docker_package.sh @@ -30,9 +30,6 @@ GENERATE_RPM=1 \ GENERATE_MSI=1 \ ENROLL_SECRET=6/EzU/+jPkxfTamWnRv1+IJsO4T9Etju \ FLEET_DESKTOP=1 \ -NUDGE=1 \ -ESCROW_BUDDY=1 \ -SWIFT_DIALOG=1 \ USE_FLEET_SERVER_CERTIFICATE=1 \ SKIP_SERVER=1 \ ./tools/tuf/test/main.sh