From 943dc41ed5935349ccf0868404fa8ebde4bc82da Mon Sep 17 00:00:00 2001 From: Tim Lee Date: Thu, 5 Mar 2026 08:37:03 -0700 Subject: [PATCH] Recovery Key password: Gitops (#40611) --- cmd/fleetctl/fleetctl/generate_gitops.go | 16 ++-- .../expectedGlobalControls.yaml | 1 + .../fleets/team-a-thumbsup.yml | 1 + .../test_dir_premium/fleets/unassigned.yml | 1 + ee/server/service/teams.go | 26 ++++-- pkg/spec/gitops.go | 11 ++- pkg/spec/gitops_test.go | 2 + pkg/spec/spec_test.go | 92 ++++++++++--------- pkg/spec/testdata/controls.yml | 1 + pkg/spec/testdata/global_config_no_paths.yml | 1 + pkg/spec/testdata/team_config_no_paths.yml | 1 + server/service/client.go | 5 + server/service/integration_mdm_test.go | 55 ++++++++++- 13 files changed, 149 insertions(+), 64 deletions(-) diff --git a/cmd/fleetctl/fleetctl/generate_gitops.go b/cmd/fleetctl/fleetctl/generate_gitops.go index b8cc60a7a5..0417dc2f3c 100644 --- a/cmd/fleetctl/fleetctl/generate_gitops.go +++ b/cmd/fleetctl/fleetctl/generate_gitops.go @@ -439,13 +439,14 @@ func (cmd *GenerateGitopsCommand) Run() error { // Set mdm to the global config by default. // We'll override this for teams other than no-team. mdmConfig := fleet.TeamMDM{ - EnableDiskEncryption: cmd.AppConfig.MDM.EnableDiskEncryption.Value, - RequireBitLockerPIN: cmd.AppConfig.MDM.RequireBitLockerPIN.Value, - MacOSUpdates: cmd.AppConfig.MDM.MacOSUpdates, - IOSUpdates: cmd.AppConfig.MDM.IOSUpdates, - IPadOSUpdates: cmd.AppConfig.MDM.IPadOSUpdates, - WindowsUpdates: cmd.AppConfig.MDM.WindowsUpdates, - MacOSSetup: cmd.AppConfig.MDM.MacOSSetup, + EnableDiskEncryption: cmd.AppConfig.MDM.EnableDiskEncryption.Value, + EnableRecoveryLockPassword: cmd.AppConfig.MDM.EnableRecoveryLockPassword.Value, + RequireBitLockerPIN: cmd.AppConfig.MDM.RequireBitLockerPIN.Value, + MacOSUpdates: cmd.AppConfig.MDM.MacOSUpdates, + IOSUpdates: cmd.AppConfig.MDM.IOSUpdates, + IPadOSUpdates: cmd.AppConfig.MDM.IPadOSUpdates, + WindowsUpdates: cmd.AppConfig.MDM.WindowsUpdates, + MacOSSetup: cmd.AppConfig.MDM.MacOSSetup, } if team == nil { @@ -1215,6 +1216,7 @@ func (cmd *GenerateGitopsCommand) generateControls(teamId *uint, teamName string if cmd.AppConfig.License.IsPremium() { if teamMdm != nil { result[jsonFieldName(mdmT, "EnableDiskEncryption")] = teamMdm.EnableDiskEncryption + result[jsonFieldName(mdmT, "EnableRecoveryLockPassword")] = teamMdm.EnableRecoveryLockPassword result[jsonFieldName(mdmT, "RequireBitLockerPIN")] = teamMdm.RequireBitLockerPIN result[jsonFieldName(mdmT, "MacOSUpdates")] = teamMdm.MacOSUpdates result[jsonFieldName(mdmT, "IOSUpdates")] = teamMdm.IOSUpdates diff --git a/cmd/fleetctl/fleetctl/testdata/generateGitops/expectedGlobalControls.yaml b/cmd/fleetctl/fleetctl/testdata/generateGitops/expectedGlobalControls.yaml index ea7c45e3d0..dad7195a72 100644 --- a/cmd/fleetctl/fleetctl/testdata/generateGitops/expectedGlobalControls.yaml +++ b/cmd/fleetctl/fleetctl/testdata/generateGitops/expectedGlobalControls.yaml @@ -39,6 +39,7 @@ enable_turn_on_windows_mdm_manually: false windows_require_bitlocker_pin: false android_enabled_and_configured: true enable_disk_encryption: true +enable_recovery_lock_password: false macos_migration: # Available in Fleet Premium enable: true mode: voluntary diff --git a/cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/fleets/team-a-thumbsup.yml b/cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/fleets/team-a-thumbsup.yml index 12ab7f04af..cbd4b289d7 100644 --- a/cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/fleets/team-a-thumbsup.yml +++ b/cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/fleets/team-a-thumbsup.yml @@ -24,6 +24,7 @@ controls: name: my_certypoo subject_name: CN=OU=$FLEET_VAR_HOST_UUID/ST=$FLEET_VAR_HOST_HARDWARE_SERIAL enable_disk_encryption: true + enable_recovery_lock_password: false ios_updates: deadline: "2021-12-31" minimum_version: "98.1" diff --git a/cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/fleets/unassigned.yml b/cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/fleets/unassigned.yml index 729df3c4ca..9c0da31cd4 100644 --- a/cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/fleets/unassigned.yml +++ b/cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/fleets/unassigned.yml @@ -1,6 +1,7 @@ controls: android_enabled_and_configured: true enable_disk_encryption: true + enable_recovery_lock_password: false enable_turn_on_windows_mdm_manually: false ios_updates: deadline: "2025-12-31" diff --git a/ee/server/service/teams.go b/ee/server/service/teams.go index 1904bbb8b9..20804dd9e7 100644 --- a/ee/server/service/teams.go +++ b/ee/server/service/teams.go @@ -1319,14 +1319,15 @@ func (svc *Service) createTeamFromSpec( AgentOptions: agentOptions, Features: features, MDM: fleet.TeamMDM{ - EnableDiskEncryption: enableDiskEncryption, - RequireBitLockerPIN: spec.MDM.RequireBitLockerPIN.Value, - MacOSUpdates: spec.MDM.MacOSUpdates, - WindowsUpdates: spec.MDM.WindowsUpdates, - MacOSSettings: macOSSettings, - MacOSSetup: macOSSetup, - WindowsSettings: spec.MDM.WindowsSettings, - AndroidSettings: spec.MDM.AndroidSettings, + EnableDiskEncryption: enableDiskEncryption, + EnableRecoveryLockPassword: spec.MDM.EnableRecoveryLockPassword.Value, + RequireBitLockerPIN: spec.MDM.RequireBitLockerPIN.Value, + MacOSUpdates: spec.MDM.MacOSUpdates, + WindowsUpdates: spec.MDM.WindowsUpdates, + MacOSSettings: macOSSettings, + MacOSSetup: macOSSetup, + WindowsSettings: spec.MDM.WindowsSettings, + AndroidSettings: spec.MDM.AndroidSettings, }, HostExpirySettings: hostExpirySettings, WebhookSettings: fleet.TeamWebhookSettings{ @@ -1463,6 +1464,15 @@ func (svc *Service) editTeamFromSpec( team.Config.MDM.RequireBitLockerPIN = spec.MDM.RequireBitLockerPIN.Value } + if spec.MDM.EnableRecoveryLockPassword.Valid { + recoveryLockPasswordUpdated := team.Config.MDM.EnableRecoveryLockPassword != spec.MDM.EnableRecoveryLockPassword.Value + if recoveryLockPasswordUpdated && !appCfg.MDM.EnabledAndConfigured { + return ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError("mdm.enable_recovery_lock_password", + `Couldn't update enable_recovery_lock_password because MDM features aren't turned on in Fleet.`)) + } + team.Config.MDM.EnableRecoveryLockPassword = spec.MDM.EnableRecoveryLockPassword.Value + } + if !team.Config.MDM.MacOSSetup.EnableReleaseDeviceManually.Valid { team.Config.MDM.MacOSSetup.EnableReleaseDeviceManually = optjson.SetBool(false) } diff --git a/pkg/spec/gitops.go b/pkg/spec/gitops.go index 4c83416cb3..ac53384c46 100644 --- a/pkg/spec/gitops.go +++ b/pkg/spec/gitops.go @@ -162,9 +162,10 @@ type GitOpsControls struct { AndroidEnabledAndConfigured any `json:"android_enabled_and_configured"` AndroidSettings any `json:"android_settings"` - EnableDiskEncryption any `json:"enable_disk_encryption"` - RequireBitLockerPIN any `json:"windows_require_bitlocker_pin,omitempty"` - Scripts []BaseItem `json:"scripts"` + EnableDiskEncryption any `json:"enable_disk_encryption"` + EnableRecoveryLockPassword any `json:"enable_recovery_lock_password"` + RequireBitLockerPIN any `json:"windows_require_bitlocker_pin,omitempty"` + Scripts []BaseItem `json:"scripts"` Defined bool } @@ -174,8 +175,8 @@ func (c GitOpsControls) Set() bool { c.IPadOSUpdates != nil || c.MacOSSettings != nil || c.MacOSSetup != nil || c.MacOSMigration != nil || c.WindowsUpdates != nil || c.WindowsSettings != nil || c.WindowsEnabledAndConfigured != nil || - c.WindowsMigrationEnabled != nil || c.EnableDiskEncryption != nil || len(c.Scripts) > 0 || - c.AndroidEnabledAndConfigured != nil || c.AndroidSettings != nil + c.WindowsMigrationEnabled != nil || c.EnableDiskEncryption != nil || c.EnableRecoveryLockPassword != nil || + len(c.Scripts) > 0 || c.AndroidEnabledAndConfigured != nil || c.AndroidSettings != nil } type Policy struct { diff --git a/pkg/spec/gitops_test.go b/pkg/spec/gitops_test.go index 8b4314ef14..d474b2de43 100644 --- a/pkg/spec/gitops_test.go +++ b/pkg/spec/gitops_test.go @@ -286,6 +286,8 @@ func TestValidGitOpsYaml(t *testing.T) { assert.True(t, ok, "windows_settings not found") _, ok = gitops.Controls.EnableDiskEncryption.(bool) assert.True(t, ok, "enable_disk_encryption not found") + _, ok = gitops.Controls.EnableRecoveryLockPassword.(bool) + assert.True(t, ok, "enable_recovery_lock_password not found") _, ok = gitops.Controls.MacOSMigration.(map[string]interface{}) assert.True(t, ok, "macos_migration not found") assert.NotNil(t, gitops.Controls.MacOSSetup, "macos_setup not found") diff --git a/pkg/spec/spec_test.go b/pkg/spec/spec_test.go index 444fe5efd6..217f2d7fad 100644 --- a/pkg/spec/spec_test.go +++ b/pkg/spec/spec_test.go @@ -291,52 +291,58 @@ Missing: $FLEET_SECRET_MISSING` } func TestGetExclusionZones(t *testing.T) { - testCases := []struct { - fixturePath []string - expected map[[2]int]string - }{ - { - []string{"testdata", "policies", "policies.yml"}, - map[[2]int]string{ - {46, 106}: " description: This policy should always fail.\n resolution:", - {93, 155}: " resolution: There is no resolution for this policy.\n query:", - {268, 328}: " description: This policy should always pass.\n resolution:", - {315, 678}: " resolution: |\n Automated method:\n Ask your system administrator to deploy the following script which will ensure proper Security Auditing Retention:\n cp /etc/security/audit_control ./tmp.txt; origExpire=$(cat ./tmp.txt | grep expire-after); sed \"s/${origExpire}/expire-after:60d OR 5G/\" ./tmp.txt > /etc/security/audit_control; rm ./tmp.txt;\n query:", - }, - }, - { - []string{"testdata", "global_config_no_paths.yml"}, - map[[2]int]string{ - {942, 1025}: " description: Collect osquery performance stats directly from osquery\n query:", // - {1830, 1894}: " description: This policy should always fail.\n resolution:", // - {1879, 1945}: " resolution: There is no resolution for this policy.\n query:", // - {2062, 2126}: " description: This policy should always pass.\n resolution:", // - {2111, 2177}: " resolution: There is no resolution for this policy.\n query:", // - {2470, 2534}: " description: This policy should always fail.\n resolution:", // - {2519, 2585}: " resolution: There is no resolution for this policy.\n query:", // - {2689, 2753}: " description: This policy should always fail.\n resolution:", // - {2738, 3111}: " resolution: |\n Automated method:\n Ask your system administrator to deploy the following script which will ensure proper Security Auditing Retention:\n cp /etc/security/audit_control ./tmp.txt; origExpire=$(cat ./tmp.txt | grep expire-after); sed \"s/${origExpire}/expire-after:60d OR 5G/\" ./tmp.txt > /etc/security/audit_control; rm ./tmp.txt;\n query:", - {6178, 6225}: " description: A cool global label\n query:", // - {6322, 6368}: " description: A fly global label\n hosts:", // - }, - }, - } + // Test with a small dedicated fixture where exact byte positions are stable + t.Run("testdata/policies/policies.yml", func(t *testing.T) { + fContents, err := os.ReadFile(filepath.Join("testdata", "policies", "policies.yml")) + require.NoError(t, err) - for _, tC := range testCases { - fPath := filepath.Join(tC.fixturePath...) + contents := string(fContents) + actual := getExclusionZones(contents) - t.Run(fPath, func(t *testing.T) { - fContents, err := os.ReadFile(fPath) - require.NoError(t, err) + expected := map[[2]int]string{ + {46, 106}: " description: This policy should always fail.\n resolution:", + {93, 155}: " resolution: There is no resolution for this policy.\n query:", + {268, 328}: " description: This policy should always pass.\n resolution:", + {315, 678}: " resolution: |\n Automated method:\n Ask your system administrator to deploy the following script which will ensure proper Security Auditing Retention:\n cp /etc/security/audit_control ./tmp.txt; origExpire=$(cat ./tmp.txt | grep expire-after); sed \"s/${origExpire}/expire-after:60d OR 5G/\" ./tmp.txt > /etc/security/audit_control; rm ./tmp.txt;\n query:", + } + require.Equal(t, len(expected), len(actual)) - contents := string(fContents) - actual := getExclusionZones(contents) - require.Equal(t, len(tC.expected), len(actual)) + for pos, text := range expected { + assert.Contains(t, actual, pos) + assert.Equal(t, contents[pos[0]:pos[1]], text, pos) + } + }) - for pos, text := range tC.expected { - assert.Contains(t, actual, pos) - assert.Equal(t, contents[pos[0]:pos[1]], text, pos) + // Test with a larger config file - verify expected text strings are found within zones + // without hardcoding byte positions (which shift when the file is modified) + t.Run("testdata/global_config_no_paths.yml", func(t *testing.T) { + fContents, err := os.ReadFile(filepath.Join("testdata", "global_config_no_paths.yml")) + require.NoError(t, err) + + contents := string(fContents) + actual := getExclusionZones(contents) + + // Expected text strings that should be found within exclusion zones + expectedTexts := []string{ + " description: Collect osquery performance stats directly from osquery\n query:", + " description: This policy should always fail.\n resolution:", + " resolution: There is no resolution for this policy.\n query:", + " description: This policy should always pass.\n resolution:", + " resolution: |\n Automated method:", + " description: A cool global label\n query:", + " description: A fly global label\n hosts:", + } + + for _, expectedText := range expectedTexts { + found := false + for _, zone := range actual { + zoneText := contents[zone[0]:zone[1]] + if zoneText == expectedText || strings.Contains(zoneText, strings.TrimPrefix(expectedText, " ")) { + found = true + break + } } - }) - } + assert.True(t, found, "expected text not found in any exclusion zone: %q", expectedText) + } + }) } diff --git a/pkg/spec/testdata/controls.yml b/pkg/spec/testdata/controls.yml index e904df4e00..9fefc96aae 100644 --- a/pkg/spec/testdata/controls.yml +++ b/pkg/spec/testdata/controls.yml @@ -7,6 +7,7 @@ windows_settings: scripts: - path: ./lib/collect-fleetd-logs.sh enable_disk_encryption: true +enable_recovery_lock_password: true macos_migration: enable: false mode: "" diff --git a/pkg/spec/testdata/global_config_no_paths.yml b/pkg/spec/testdata/global_config_no_paths.yml index 9fa5f1553a..606404d8c5 100644 --- a/pkg/spec/testdata/global_config_no_paths.yml +++ b/pkg/spec/testdata/global_config_no_paths.yml @@ -9,6 +9,7 @@ controls: # Controls added to "No team" scripts: - path: ./lib/collect-fleetd-logs.sh enable_disk_encryption: true + enable_recovery_lock_password: true macos_migration: enable: false mode: "" diff --git a/pkg/spec/testdata/team_config_no_paths.yml b/pkg/spec/testdata/team_config_no_paths.yml index aa9bdc7360..50c57ef919 100644 --- a/pkg/spec/testdata/team_config_no_paths.yml +++ b/pkg/spec/testdata/team_config_no_paths.yml @@ -39,6 +39,7 @@ controls: scripts: - path: ./lib/collect-fleetd-logs.sh enable_disk_encryption: true + enable_recovery_lock_password: true windows_require_bitlocker_pin: true macos_setup: bootstrap_package: null diff --git a/server/service/client.go b/server/service/client.go index fee3f581ed..872b0b0d76 100644 --- a/server/service/client.go +++ b/server/service/client.go @@ -2297,10 +2297,14 @@ func (c *Client) DoGitOps( // Put in default value for enable_disk_encryption enableDiskEncryption := false + enableRecoveryLockPassword := false requireBitLockerPIN := false if incoming.Controls.EnableDiskEncryption != nil { enableDiskEncryption = incoming.Controls.EnableDiskEncryption.(bool) } + if incoming.Controls.EnableRecoveryLockPassword != nil { + enableRecoveryLockPassword = incoming.Controls.EnableRecoveryLockPassword.(bool) + } if incoming.Controls.RequireBitLockerPIN != nil { requireBitLockerPIN = incoming.Controls.RequireBitLockerPIN.(bool) } @@ -2309,6 +2313,7 @@ func (c *Client) DoGitOps( } mdmAppConfig["enable_disk_encryption"] = enableDiskEncryption + mdmAppConfig["enable_recovery_lock_password"] = enableRecoveryLockPassword mdmAppConfig["windows_require_bitlocker_pin"] = requireBitLockerPIN if incoming.TeamName != nil { diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index d084de74b2..96922fee6b 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -787,7 +787,6 @@ func (s *integrationMDMTestSuite) SetupSuite() { s.T().Setenv("FLEET_DEV_AZURE_JWT_JWKS_URI", jwksServer.URL+"/jwks.json") dev_mode.SetOverride("FLEET_DEV_BATCH_RETRY_INTERVAL", "1s") - } func (s *integrationMDMTestSuite) TearDownSuite() { @@ -3409,6 +3408,52 @@ func (s *integrationMDMTestSuite) TestTeamsMDMRecoveryLockPassword() { teamResp = getTeamResponse{} s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/teams/%d", team.ID), nil, http.StatusOK, &teamResp) require.False(t, teamResp.Team.Config.MDM.EnableRecoveryLockPassword) + + // test via apply team specs + teamSpecs := applyTeamSpecsRequest{Specs: []*fleet.TeamSpec{{ + Name: teamName, + MDM: fleet.TeamSpecMDM{ + EnableRecoveryLockPassword: optjson.SetBool(true), + }, + }}} + s.Do("POST", "/api/latest/fleet/spec/teams", teamSpecs, http.StatusOK) + + // check it's enabled + teamResp = getTeamResponse{} + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/teams/%d", team.ID), nil, http.StatusOK, &teamResp) + require.True(t, teamResp.Team.Config.MDM.EnableRecoveryLockPassword) + + // apply with recovery lock password disabled + teamSpecs = applyTeamSpecsRequest{Specs: []*fleet.TeamSpec{{ + Name: teamName, + MDM: fleet.TeamSpecMDM{ + EnableRecoveryLockPassword: optjson.SetBool(false), + }, + }}} + s.Do("POST", "/api/latest/fleet/spec/teams", teamSpecs, http.StatusOK) + + // check it's disabled + teamResp = getTeamResponse{} + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/teams/%d", team.ID), nil, http.StatusOK, &teamResp) + require.False(t, teamResp.Team.Config.MDM.EnableRecoveryLockPassword) + + // create a new team via spec with recovery lock password enabled + newTeamName := teamName + "2" + teamSpecs = applyTeamSpecsRequest{Specs: []*fleet.TeamSpec{{ + Name: newTeamName, + MDM: fleet.TeamSpecMDM{ + EnableRecoveryLockPassword: optjson.SetBool(true), + }, + }}} + var specResp applyTeamSpecsResponse + s.DoJSON("POST", "/api/latest/fleet/spec/teams", teamSpecs, http.StatusOK, &specResp) + newTeamID := specResp.TeamIDsByName[newTeamName] + require.NotZero(t, newTeamID) + + // check the new team has recovery lock password enabled + teamResp = getTeamResponse{} + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/teams/%d", newTeamID), nil, http.StatusOK, &teamResp) + require.True(t, teamResp.Team.Config.MDM.EnableRecoveryLockPassword) } func (s *integrationMDMTestSuite) TestEnrollOrbitAfterDEPSync() { @@ -10448,6 +10493,14 @@ func (s *integrationMDMTestSuite) TestMDMEnabledAndConfigured() { // disk encryption does not require mdm enabled and configured http.StatusOK, }, + { + "enable recovery lock password", + &fleet.TeamSpecMDM{ + EnableRecoveryLockPassword: optjson.SetBool(true), + }, + // recovery lock password requires mdm enabled and configured + http.StatusUnprocessableEntity, + }, // Ian - this test still passes, that is, returns 4xx – perhaps related to one of the endpoints we still need to update { "enable end user auth",