From 56c0773ff39a76dbc9b948adeea92b92ae794d29 Mon Sep 17 00:00:00 2001 From: Juan Fernandez Date: Tue, 26 Aug 2025 13:23:24 -0400 Subject: [PATCH] Validate Require BitLocker PIN (#32240) For #28133 Validate that if 'Require BitLocker PIN' is enabled, disk encryption must be enabled as well. --- ee/server/service/teams.go | 7 ++ ee/server/service/teams_test.go | 83 +++++++++++++++++++ .../cards/DiskEncryption/DiskEncryption.tsx | 8 +- server/fleet/errors.go | 18 ++-- server/service/appconfig.go | 16 ++++ server/service/appconfig_test.go | 41 ++++++++- server/service/client.go | 20 +++-- 7 files changed, 172 insertions(+), 21 deletions(-) diff --git a/ee/server/service/teams.go b/ee/server/service/teams.go index 385ad9991d..a531d6bbdd 100644 --- a/ee/server/service/teams.go +++ b/ee/server/service/teams.go @@ -1669,6 +1669,13 @@ func (svc *Service) updateTeamMDMDiskEncryption(ctx context.Context, tm *fleet.T } if didUpdateEncryption || didUpdateRequirePIN { + if didUpdateEncryption && !tm.Config.MDM.EnableDiskEncryption && tm.Config.MDM.RequireBitLockerPIN { + return ctxerr.New(ctx, fleet.CantDisableDiskEncryptionIfPINRequiredErrMsg) + } + if !didUpdateEncryption && !tm.Config.MDM.EnableDiskEncryption && tm.Config.MDM.RequireBitLockerPIN { + return ctxerr.New(ctx, fleet.CantEnablePINRequiredIfDiskEncryptionEnabled) + } + if _, err := svc.ds.SaveTeam(ctx, tm); err != nil { return err } diff --git a/ee/server/service/teams_test.go b/ee/server/service/teams_test.go index 6225899b46..0f18e856fc 100644 --- a/ee/server/service/teams_test.go +++ b/ee/server/service/teams_test.go @@ -1,13 +1,96 @@ package service import ( + "context" + "strings" "testing" + "github.com/fleetdm/fleet/v4/server/config" "github.com/fleetdm/fleet/v4/server/fleet" + "github.com/fleetdm/fleet/v4/server/mock" "github.com/fleetdm/fleet/v4/server/ptr" "github.com/stretchr/testify/require" ) +func TestUpdateTeamMDMDiskEncryption(t *testing.T) { + testCases := []struct { + name string + mdmConfig fleet.TeamMDM + diskEncryption *bool + requireTPMPIN *bool + expectedError string + }{ + { + name: "try to disable disk encryption with TPM PIN enabled", + mdmConfig: fleet.TeamMDM{ + EnableDiskEncryption: true, + RequireBitLockerPIN: true, + }, + diskEncryption: ptr.Bool(false), + requireTPMPIN: ptr.Bool(true), + + expectedError: fleet.CantDisableDiskEncryptionIfPINRequiredErrMsg, + }, + { + name: "try to enable disk encryption with TPM PIN enabled", + mdmConfig: fleet.TeamMDM{ + EnableDiskEncryption: false, + RequireBitLockerPIN: false, + }, + diskEncryption: ptr.Bool(false), + requireTPMPIN: ptr.Bool(true), + expectedError: fleet.CantEnablePINRequiredIfDiskEncryptionEnabled, + }, + { + name: "try to disable disk encryption with TPM PIN enabled when disk encryption prev enabled", + mdmConfig: fleet.TeamMDM{ + EnableDiskEncryption: true, + RequireBitLockerPIN: false, + }, + diskEncryption: ptr.Bool(false), + requireTPMPIN: ptr.Bool(true), + expectedError: fleet.CantDisableDiskEncryptionIfPINRequiredErrMsg, + }, + } + + ds := new(mock.Store) + + svc := &Service{ + ds: ds, + config: config.FleetConfig{ + Server: config.ServerConfig{ + PrivateKey: "something", + }, + }, + } + + ctx := context.Background() + + for _, tC := range testCases { + team := fleet.Team{ + Config: fleet.TeamConfig{ + MDM: tC.mdmConfig, + }, + } + + err := svc.updateTeamMDMDiskEncryption( + ctx, + &team, + tC.diskEncryption, + tC.requireTPMPIN, + ) + + if tC.expectedError != "" { + require.NotNil(t, err) + require.True( + t, + strings.Contains(err.Error(), tC.expectedError), + "Expected '%s' to contain '%s'", + err.Error(), tC.expectedError) + } + } +} + func TestObfuscateSecrets(t *testing.T) { buildTeams := func(n int) []*fleet.Team { r := make([]*fleet.Team, 0, n) diff --git a/frontend/pages/ManageControlsPage/OSSettings/cards/DiskEncryption/DiskEncryption.tsx b/frontend/pages/ManageControlsPage/OSSettings/cards/DiskEncryption/DiskEncryption.tsx index 391f5b3ea7..56c64a31b0 100644 --- a/frontend/pages/ManageControlsPage/OSSettings/cards/DiskEncryption/DiskEncryption.tsx +++ b/frontend/pages/ManageControlsPage/OSSettings/cards/DiskEncryption/DiskEncryption.tsx @@ -126,10 +126,10 @@ const DiskEncryption = ({ ); } else { - renderFlash( - "error", - "Could not update the disk encryption enforcement. Please try again." - ); + const errorMsg = + getErrorReason(e) ?? + "Could not update the disk encryption enforcement. Please try again."; + renderFlash("error", errorMsg); } } }; diff --git a/server/fleet/errors.go b/server/fleet/errors.go index bda944e40e..7dfa40f0cb 100644 --- a/server/fleet/errors.go +++ b/server/fleet/errors.go @@ -22,14 +22,16 @@ var ( ErrWindowsMDMNotConfigured = &WindowsMDMNotConfiguredError{} ErrNotConfigured = &NotConfiguredError{} - MDMNotConfiguredMessage = "MDM features aren't turned on in Fleet. For more information about setting up MDM, please visit https://fleetdm.com/docs/using-fleet" - WindowsMDMNotConfiguredMessage = "Windows MDM isn't turned on. For more information about setting up MDM, please visit https://fleetdm.com/learn-more-about/windows-mdm" - AppleMDMNotConfiguredMessage = "macOS MDM isn't turned on. Visit https://fleetdm.com/docs/using-fleet to learn how to turn on MDM." - AppleABMDefaultTeamDeprecatedMessage = "mdm.apple_bm_default_team has been deprecated. Please use the new mdm.apple_business_manager key documented here: https://fleetdm.com/learn-more-about/apple-business-manager-gitops" - CantTurnOffMDMForWindowsHostsMessage = "Can't turn off MDM for Windows hosts." - CantTurnOffMDMForPersonalHostsMessage = "Couldn't turn off MDM. This command isn't available for personal hosts." - CantWipePersonalHostsMessage = "Couldn't wipe. This command isn't available for personal hosts." - CantLockPersonalHostsMessage = "Couldn't lock. This command isn't available for personal hosts." + MDMNotConfiguredMessage = "MDM features aren't turned on in Fleet. For more information about setting up MDM, please visit https://fleetdm.com/docs/using-fleet" + WindowsMDMNotConfiguredMessage = "Windows MDM isn't turned on. For more information about setting up MDM, please visit https://fleetdm.com/learn-more-about/windows-mdm" + AppleMDMNotConfiguredMessage = "macOS MDM isn't turned on. Visit https://fleetdm.com/docs/using-fleet to learn how to turn on MDM." + AppleABMDefaultTeamDeprecatedMessage = "mdm.apple_bm_default_team has been deprecated. Please use the new mdm.apple_business_manager key documented here: https://fleetdm.com/learn-more-about/apple-business-manager-gitops" + CantTurnOffMDMForWindowsHostsMessage = "Can't turn off MDM for Windows hosts." + CantTurnOffMDMForPersonalHostsMessage = "Couldn't turn off MDM. This command isn't available for personal hosts." + CantWipePersonalHostsMessage = "Couldn't wipe. This command isn't available for personal hosts." + CantLockPersonalHostsMessage = "Couldn't lock. This command isn't available for personal hosts." + CantDisableDiskEncryptionIfPINRequiredErrMsg = "Couldn't disable disk encryption, you need to disable the BitLocker PIN requirement first." + CantEnablePINRequiredIfDiskEncryptionEnabled = "Couldn't enable BitLocker PIN requirement, you must enable disk encryption first." ) // ErrWithStatusCode is an interface for errors that should set a specific HTTP diff --git a/server/service/appconfig.go b/server/service/appconfig.go index 074a4c0759..525a6fc965 100644 --- a/server/service/appconfig.go +++ b/server/service/appconfig.go @@ -1751,6 +1751,22 @@ func (svc *Service) validateMDM( if !mdm.WindowsEnabledAndConfigured && mdm.WindowsMigrationEnabled { invalid.Append("mdm.windows_migration_enabled", "Couldn't enable Windows MDM migration, Windows MDM is not enabled.") } + + if !mdm.EnableDiskEncryption.Value { + switch { + case !oldMdm.EnableDiskEncryption.Value && mdm.RequireBitLockerPIN.Value: + invalid.Append( + "mdm.windows_require_bitlocker_pin", + fleet.CantEnablePINRequiredIfDiskEncryptionEnabled, + ) + case oldMdm.EnableDiskEncryption.Value && mdm.RequireBitLockerPIN.Value: + invalid.Append( + "mdm.enable_disk_encryption", + fleet.CantDisableDiskEncryptionIfPINRequiredErrMsg, + ) + } + } + return nil } diff --git a/server/service/appconfig_test.go b/server/service/appconfig_test.go index 9c0b9086ba..57a782d644 100644 --- a/server/service/appconfig_test.go +++ b/server/service/appconfig_test.go @@ -901,7 +901,7 @@ func TestTransparencyURLDowngradeLicense(t *testing.T) { require.Equal(t, "", ac.FleetDesktop.TransparencyURL) } -func TestMDMAppleConfig(t *testing.T) { +func TestMDMConfig(t *testing.T) { ds := new(mock.Store) depStorage := new(nanodep_mock.Storage) @@ -1179,6 +1179,45 @@ func TestMDMAppleConfig(t *testing.T) { RequireBitLockerPIN: optjson.Bool{Set: true, Value: false}, }, }, + { + name: "try to disable disk encryption with TPM PIN enabled", + licenseTier: "premium", + oldMDM: fleet.MDM{ + EnableDiskEncryption: optjson.SetBool(true), + RequireBitLockerPIN: optjson.SetBool(true), + }, + newMDM: fleet.MDM{ + EnableDiskEncryption: optjson.SetBool(false), + RequireBitLockerPIN: optjson.SetBool(true), + }, + expectedError: fleet.CantDisableDiskEncryptionIfPINRequiredErrMsg, + }, + { + name: "try to enable disk encryption with TPM PIN enabled", + licenseTier: "premium", + oldMDM: fleet.MDM{ + EnableDiskEncryption: optjson.SetBool(false), + RequireBitLockerPIN: optjson.SetBool(false), + }, + newMDM: fleet.MDM{ + EnableDiskEncryption: optjson.SetBool(false), + RequireBitLockerPIN: optjson.SetBool(true), + }, + expectedError: fleet.CantEnablePINRequiredIfDiskEncryptionEnabled, + }, + { + name: "try to disable disk encryption with TPM PIN enabled when disk encryption prev enabled", + licenseTier: "premium", + oldMDM: fleet.MDM{ + EnableDiskEncryption: optjson.SetBool(true), + RequireBitLockerPIN: optjson.SetBool(false), + }, + newMDM: fleet.MDM{ + EnableDiskEncryption: optjson.SetBool(false), + RequireBitLockerPIN: optjson.SetBool(true), + }, + expectedError: fleet.CantDisableDiskEncryptionIfPINRequiredErrMsg, + }, } for _, tt := range testCases { diff --git a/server/service/client.go b/server/service/client.go index 782ccd7a14..aac9b9e95b 100644 --- a/server/service/client.go +++ b/server/service/client.go @@ -1637,8 +1637,7 @@ func (c *Client) DoGitOps( logf func(format string, args ...interface{}), dryRun bool, teamDryRunAssumptions *fleet.TeamSpecsDryRunAssumptions, - appConfig *fleet.EnrichedAppConfig, - // pass-by-ref to build lists + appConfig *fleet.EnrichedAppConfig, // pass-by-ref to build lists teamsSoftwareInstallers map[string][]fleet.SoftwarePackageResponse, teamsVPPApps map[string][]fleet.VPPAppResponse, teamsScripts map[string][]fleet.ScriptResponse, @@ -2046,17 +2045,22 @@ func (c *Client) DoGitOps( windowsUpdates["grace_period_days"] = nil } } + // Put in default value for enable_disk_encryption + enableDiskEncryption := false + requireBitLockerPIN := false if config.Controls.EnableDiskEncryption != nil { - mdmAppConfig["enable_disk_encryption"] = config.Controls.EnableDiskEncryption - } else { - mdmAppConfig["enable_disk_encryption"] = false + enableDiskEncryption = config.Controls.EnableDiskEncryption.(bool) } if config.Controls.RequireBitLockerPIN != nil { - mdmAppConfig["windows_require_bitlocker_pin"] = config.Controls.RequireBitLockerPIN - } else { - mdmAppConfig["windows_require_bitlocker_pin"] = false + requireBitLockerPIN = config.Controls.RequireBitLockerPIN.(bool) } + if !enableDiskEncryption && requireBitLockerPIN { + return nil, nil, errors.New("enable_disk_encryption cannot be false if windows_require_bitlocker_pin is true") + } + + mdmAppConfig["enable_disk_encryption"] = enableDiskEncryption + mdmAppConfig["windows_require_bitlocker_pin"] = requireBitLockerPIN if config.TeamName != nil { team["gitops_filename"] = filename