make sure we report the correct error during BitLocker encryption (#16096)

for #15916, explanation of the rationale in the description of
`isMisreportedDecryptionError` and in the issue comments.

I refactored the code a little bit, trying to make it easier to follow
even with the added complexity.

This also paves the road for #15711

orbit/changes/15916-bitlocker-message

@@ -0,0 +1 @@
+* Fixed an issue that would cause `fleetd` to report the wrong error if BitLocker encryption fails.

orbit/pkg/bitlocker/bitlocker_management.go

@@ -1,17 +1,94 @@
 package bitlocker
 
-// Encryption Status
+// Volume encryption/decryption status.
-type EncryptionStatus struct {
+//
-	ProtectionStatusDesc string
+// Values and their meanings were taken from:
-	ConversionStatusDesc string
+// https://learn.microsoft.com/en-us/windows/win32/secprov/getconversionstatus-win32-encryptablevolume
-	EncryptionPercentage string
+const (
-	EncryptionFlags      string
+	ConversionStatusFullyDecrypted       int32 = 0
-	WipingStatusDesc     string
+	ConversionStatusFullyEncrypted       int32 = 1
-	WipingPercentage     string
+	ConversionStatusEncryptionInProgress int32 = 2
+	ConversionStatusDecryptionInProgress int32 = 3
+	ConversionStatusEncryptionPaused     int32 = 4
+	ConversionStatusDecryptionPaused     int32 = 5
+)
+
+// Free space wiping status.
+//
+// Values and their meanings were taken from:
+// https://learn.microsoft.com/en-us/windows/win32/secprov/getconversionstatus-win32-encryptablevolume
+const (
+	WipingStatusFreeSpaceNotWiped         int32 = 0
+	WipingStatusFreeSpaceWiped            int32 = 1
+	WipingStatusFreeSpaceWipingInProgress int32 = 2
+	WipingStatusFreeSpaceWipingPaused     int32 = 3
+)
+
+// Specifies whether the volume and the encryption key (if any) are secured.
+//
+// Values and their meanings were taken from:
+// https://learn.microsoft.com/en-us/windows/win32/secprov/getprotectionstatus-win32-encryptablevolume
+const (
+	ProtectionStatusUnprotected int32 = 0
+	ProtectionStatusProtected   int32 = 1
+	ProtectionStatusUnknown     int32 = 2
+)
+
+const (
+	// Error Codes
+	ErrorCodeIODevice                   int32 = -2147023779
+	ErrorCodeDriveIncompatibleVolume    int32 = -2144272206
+	ErrorCodeNoTPMWithPassphrase        int32 = -2144272212
+	ErrorCodePassphraseTooLong          int32 = -2144272214
+	ErrorCodePolicyPassphraseNotAllowed int32 = -2144272278
+	ErrorCodeNotDecrypted               int32 = -2144272327
+	ErrorCodeInvalidPasswordFormat      int32 = -2144272331
+	ErrorCodeBootableCDOrDVD            int32 = -2144272336
+	ErrorCodeProtectorExists            int32 = -2144272335
+)
+
+// EncryptionError represents an error that occurs during the encryption
+// process.
+type EncryptionError struct {
+	msg  string // msg is the error message describing what went wrong.
+	code int32  // code is the Bitlocker-specific error code.
 }
 
-// Volume Encryption Status
+func NewEncryptionError(msg string, code int32) *EncryptionError {
-type VolumeStatus struct {
+	return &EncryptionError{
-	DriveVolume string
+		msg:  msg,
-	Status      *EncryptionStatus
+		code: code,
+	}
+}
+
+// Error returns the error message of the EncryptionError.
+// This method makes EncryptionError compatible with the Go built-in error
+// interface.
+func (e *EncryptionError) Error() string {
+	return e.msg
+}
+
+// Code returns the Bitlocker-specific error code.
+// These codes are defined by Microsoft and are used to identify specific types
+// of encryption errors.
+func (e *EncryptionError) Code() int32 {
+	return e.code
+}
+
+// EncryptionStatus represents the encryption status of a volume as returned by
+// the GetConversionStatus method of the Win32_EncryptableVolume class.
+type EncryptionStatus struct {
+	ProtectionStatus     int32  // indicates whether the volume and its encryption key are secured.
+	ConversionStatus     int32  // represents the encryption or decryption status of the volume.
+	EncryptionPercentage string // percentage of the volume that is encrypted.
+	EncryptionFlags      string // flags describing the encryption behavior.
+	WipingStatus         int32  // status of the free space wiping on the volume.
+	WipingPercentage     string // percentage of free space that has been wiped.
+}
+
+// VolumeStatus provides the encryption status for a specific drive volume.
+// It ties a volume (identified by its drive letter) to its EncryptionStatus.
+type VolumeStatus struct {
+	DriveVolume string            // driveVolume is the identifier of the drive (e.g., "C:").
+	Status      *EncryptionStatus // status holds the encryption status of the volume.
 }

orbit/pkg/bitlocker/bitlocker_management_windows.go

@@ -34,17 +34,6 @@ const (
 	EncryptDataOnly    EncryptionFlag = 0x00000001
 	EncryptDemandWipe  EncryptionFlag = 0x00000002
 	EncryptSynchronous EncryptionFlag = 0x00010000
-
-	// Error Codes
-	ERROR_IO_DEVICE                     int32 = -2147023779
-	FVE_E_EDRIVE_INCOMPATIBLE_VOLUME    int32 = -2144272206
-	FVE_E_NO_TPM_WITH_PASSPHRASE        int32 = -2144272212
-	FVE_E_PASSPHRASE_TOO_LONG           int32 = -2144272214
-	FVE_E_POLICY_PASSPHRASE_NOT_ALLOWED int32 = -2144272278
-	FVE_E_NOT_DECRYPTED                 int32 = -2144272327
-	FVE_E_INVALID_PASSWORD_FORMAT       int32 = -2144272331
-	FVE_E_BOOTABLE_CDDVD                int32 = -2144272336
-	FVE_E_PROTECTOR_EXISTS              int32 = -2144272335
 )
 
 // DiscoveryVolumeType specifies the type of discovery volume to be used by Prepare.
@@ -74,28 +63,32 @@ const (
 )
 
 func encryptErrHandler(val int32) error {
+	var msg string
+
 	switch val {
-	case ERROR_IO_DEVICE:
+	case ErrorCodeIODevice:
-		return fmt.Errorf("an I/O error has occurred during encryption; the device may need to be reset")
+		msg = "an I/O error has occurred during encryption; the device may need to be reset"
-	case FVE_E_EDRIVE_INCOMPATIBLE_VOLUME:
+	case ErrorCodeDriveIncompatibleVolume:
-		return fmt.Errorf("the drive specified does not support hardware-based encryption")
+		msg = "the drive specified does not support hardware-based encryption"
-	case FVE_E_NO_TPM_WITH_PASSPHRASE:
+	case ErrorCodeNoTPMWithPassphrase:
-		return fmt.Errorf("a TPM key protector cannot be added because a password protector exists on the drive")
+		msg = "a TPM key protector cannot be added because a password protector exists on the drive"
-	case FVE_E_PASSPHRASE_TOO_LONG:
+	case ErrorCodePassphraseTooLong:
-		return fmt.Errorf("the passphrase cannot exceed 256 characters")
+		msg = "the passphrase cannot exceed 256 characters"
-	case FVE_E_POLICY_PASSPHRASE_NOT_ALLOWED:
+	case ErrorCodePolicyPassphraseNotAllowed:
-		return fmt.Errorf("group Policy settings do not permit the creation of a password")
+		msg = "group Policy settings do not permit the creation of a password"
-	case FVE_E_NOT_DECRYPTED:
+	case ErrorCodeNotDecrypted:
-		return fmt.Errorf("the drive must be fully decrypted to complete this operation")
+		msg = "the drive must be fully decrypted to complete this operation"
-	case FVE_E_INVALID_PASSWORD_FORMAT:
+	case ErrorCodeInvalidPasswordFormat:
-		return fmt.Errorf("the format of the recovery password provided is invalid")
+		msg = "the format of the recovery password provided is invalid"
-	case FVE_E_BOOTABLE_CDDVD:
+	case ErrorCodeBootableCDOrDVD:
-		return fmt.Errorf("bitLocker Drive Encryption detected bootable media (CD or DVD) in the computer")
+		msg = "BitLocker Drive Encryption detected bootable media (CD or DVD) in the computer"
-	case FVE_E_PROTECTOR_EXISTS:
+	case ErrorCodeProtectorExists:
-		return fmt.Errorf("key protector cannot be added; only one key protector of this type is allowed for this drive")
+		msg = "key protector cannot be added; only one key protector of this type is allowed for this drive"
 	default:
-		return fmt.Errorf("error code returned during encryption: %d", val)
+		msg = "error code returned during encryption: %d"
 	}
+
+	return &EncryptionError{msg, val}
 }
 
 /////////////////////////////////////////////////////
@@ -263,11 +256,11 @@ func (v *Volume) getBitlockerStatus() (*EncryptionStatus, error) {
 
 	// Creating the encryption status struct
 	encStatus := &EncryptionStatus{
-		ProtectionStatusDesc: getProtectionStatusDescription(fmt.Sprintf("%d", protectionStatus)),
+		ProtectionStatus:     protectionStatus,
-		ConversionStatusDesc: getConversionStatusDescription(fmt.Sprintf("%d", conversionStatus)),
+		ConversionStatus:     conversionStatus,
 		EncryptionPercentage: intToPercentage(encryptionPercentage),
 		EncryptionFlags:      fmt.Sprintf("%d", encryptionFlags),
-		WipingStatusDesc:     getWipingStatusDescription(fmt.Sprintf("%d", wipingStatus)),
+		WipingStatus:         wipingStatus,
 		WipingPercentage:     intToPercentage(wipingPercentage),
 	}
 
@@ -344,59 +337,6 @@ func bitlockerConnect(driveLetter string) (Volume, error) {
 	return v, nil
 }
 
-// getConversionStatusDescription returns the current status of the volume
-// https://learn.microsoft.com/en-us/windows/win32/secprov/getconversionstatus-win32-encryptablevolume
-func getConversionStatusDescription(input string) string {
-	switch input {
-	case "0":
-		return "FullyDecrypted"
-	case "1":
-		return "FullyEncrypted"
-	case "2":
-		return "EncryptionInProgress"
-	case "3":
-		return "DecryptionInProgress"
-	case "4":
-		return "EncryptionPaused"
-	case "5":
-		return "DecryptionPaused"
-	}
-
-	return "Status " + input
-}
-
-// getWipingStatusDescription returns the current wiping status of the volume
-// https://learn.microsoft.com/en-us/windows/win32/secprov/getconversionstatus-win32-encryptablevolume
-func getWipingStatusDescription(input string) string {
-	switch input {
-	case "0":
-		return "FreeSpaceNotWiped"
-	case "1":
-		return "FreeSpaceWiped"
-	case "2":
-		return "FreeSpaceWipingInProgress"
-	case "3":
-		return "FreeSpaceWipingPaused"
-	}
-
-	return "Status " + input
-}
-
-// getProtectionStatusDescription returns the current protection status of the volume
-// https://learn.microsoft.com/en-us/windows/win32/secprov/getprotectionstatus-win32-encryptablevolume
-func getProtectionStatusDescription(input string) string {
-	switch input {
-	case "0":
-		return "Unprotected"
-	case "1":
-		return "Protected"
-	case "2":
-		return "Unknown"
-	}
-
-	return "Status " + input
-}
-
 // intToPercentage converts an int to a percentage string
 func intToPercentage(num int32) string {
 	percentage := float64(num) / 10000.0
@@ -446,18 +386,18 @@ func bitsToDrives(bitMap uint32) (drives []string) {
 func getLogicalVolumes() ([]string, error) {
 	kernel32, err := syscall.LoadLibrary("kernel32.dll")
 	if err != nil {
-		return nil, fmt.Errorf("failed to load kernel32.dll: %v", err)
+		return nil, fmt.Errorf("failed to load kernel32.dll: %w", err)
 	}
 	defer syscall.FreeLibrary(kernel32)
 
 	getLogicalDrivesHandle, err := syscall.GetProcAddress(kernel32, "GetLogicalDrives")
 	if err != nil {
-		return nil, fmt.Errorf("failed to get procedure address: %v", err)
+		return nil, fmt.Errorf("failed to get procedure address: %w", err)
 	}
 
 	ret, _, callErr := syscall.SyscallN(uintptr(getLogicalDrivesHandle), 0, 0, 0, 0)
 	if callErr != 0 {
-		return nil, fmt.Errorf("syscall to GetLogicalDrives failed: %v", callErr)
+		return nil, fmt.Errorf("syscall to GetLogicalDrives failed: %w", callErr)
 	}
 
 	return bitsToDrives(uint32(ret)), nil
@@ -467,14 +407,14 @@ func getBitlockerStatus(targetVolume string) (*EncryptionStatus, error) {
 	// Connect to the volume
 	vol, err := bitlockerConnect(targetVolume)
 	if err != nil {
-		return nil, fmt.Errorf("there was an error connecting to the volume - error: %v", err)
+		return nil, fmt.Errorf("connecting to the volume: %w", err)
 	}
 	defer vol.bitlockerClose()
 
 	// Get volume status
 	status, err := vol.getBitlockerStatus()
 	if err != nil {
-		return nil, fmt.Errorf("there was an error starting decryption - error: %v", err)
+		return nil, fmt.Errorf("starting decryption: %w", err)
 	}
 
 	return status, nil
@@ -488,14 +428,14 @@ func GetRecoveryKeys(targetVolume string) (map[string]string, error) {
 	// Connect to the volume
 	vol, err := bitlockerConnect(targetVolume)
 	if err != nil {
-		return nil, fmt.Errorf("there was an error connecting to the volume - error: %v", err)
+		return nil, fmt.Errorf("connecting to the volume: %w", err)
 	}
 	defer vol.bitlockerClose()
 
 	// Get recovery keys
 	keys, err := vol.getProtectorsKeys()
 	if err != nil {
-		return nil, fmt.Errorf("there was an error retreving protection keys: %v", err)
+		return nil, fmt.Errorf("retreving protection keys: %w", err)
 	}
 
 	return keys, nil
@@ -505,29 +445,29 @@ func EncryptVolume(targetVolume string) (string, error) {
 	// Connect to the volume
 	vol, err := bitlockerConnect(targetVolume)
 	if err != nil {
-		return "", fmt.Errorf("there was an error connecting to the volume - error: %v", err)
+		return "", fmt.Errorf("connecting to the volume: %w", err)
 	}
 	defer vol.bitlockerClose()
 
 	// Prepare for encryption
 	if err := vol.prepareVolume(VolumeTypeDefault, EncryptionTypeSoftware); err != nil {
-		return "", fmt.Errorf("there was an error preparing the volume for encryption - error: %v", err)
+		return "", fmt.Errorf("preparing volume for encryption: %w", err)
 	}
 
 	// Add a recovery protector
 	recoveryKey, err := vol.protectWithNumericalPassword()
 	if err != nil {
-		return "", fmt.Errorf("there was an error adding a recovery protector - error: %v", err)
+		return "", fmt.Errorf("adding a recovery protector: %w", err)
 	}
 
 	// Protect with TPM
 	if err := vol.protectWithTPM(nil); err != nil {
-		return "", fmt.Errorf("there was an error protecting with TPM - error: %v", err)
+		return "", fmt.Errorf("protecting with TPM: %w", err)
 	}
 
 	// Start encryption
 	if err := vol.encrypt(XtsAES256, EncryptDataOnly); err != nil {
-		return "", fmt.Errorf("there was an error starting encryption - error: %v", err)
+		return "", fmt.Errorf("starting encryption: %w", err)
 	}
 
 	return recoveryKey, nil
@@ -537,13 +477,13 @@ func DecryptVolume(targetVolume string) error {
 	// Connect to the volume
 	vol, err := bitlockerConnect(targetVolume)
 	if err != nil {
-		return fmt.Errorf("there was an error connecting to the volume - error: %v", err)
+		return fmt.Errorf("connecting to the volume: %w", err)
 	}
 	defer vol.bitlockerClose()
 
 	// Start decryption
 	if err := vol.decrypt(); err != nil {
-		return fmt.Errorf("there was an error starting decryption - error: %v", err)
+		return fmt.Errorf("starting decryption: %w", err)
 	}
 
 	return nil
@@ -552,7 +492,7 @@ func DecryptVolume(targetVolume string) error {
 func GetEncryptionStatus() ([]VolumeStatus, error) {
 	drives, err := getLogicalVolumes()
 	if err != nil {
-		return nil, fmt.Errorf("logical volumen enumeration %v", err)
+		return nil, fmt.Errorf("logical volumen enumeration %w", err)
 	}
 
 	// iterate drives

orbit/pkg/update/notifications.go

@@ -403,7 +403,18 @@ type DiskEncryptionKeySetter interface {
 	SetOrUpdateDiskEncryptionKey(diskEncryptionStatus fleet.OrbitHostDiskEncryptionKeyPayload) error
 }
 
-type execEncryptVolumeFunc func(string) (string, error)
+// execEncryptVolumeFunc handles the encryption of a volume identified by its
+// string identifier (e.g., "C:").
+//
+// It returns a string representing the recovery key and an error if any occurs during the process.
+type execEncryptVolumeFunc func(volumeID string) (recoveryKey string, err error)
+
+// execGetEncryptionStatusFunc retrieves the encryption status of all volumes
+// managed by Bitlocker.
+//
+// It returns a slice of bitlocker.VolumeStatus, each representing the
+// encryption status of a volume, and an error if the operation fails.
+type execGetEncryptionStatusFunc func() (status []bitlocker.VolumeStatus, err error)
 
 type windowsMDMBitlockerConfigFetcher struct {
 	// Fetcher is the OrbitConfigFetcher that will be wrapped. It is responsible
@@ -417,15 +428,19 @@ type windowsMDMBitlockerConfigFetcher struct {
 	// Bitlocker Operation Results
 	EncryptionResult DiskEncryptionKeySetter
 
-	// tracks last time the enrollment command was executed
+	// tracks last time a disk encryption has successfully run
-	lastEnrollRun time.Time
+	lastRun time.Time
 
 	// ensures only one script execution runs at a time
 	mu sync.Mutex
 
 	// for tests, to be able to mock API commands. If nil, will use
-	// EncryptVolume
+	// bitlocker.EncryptVolume
 	execEncryptVolumeFn execEncryptVolumeFunc
+
+	// for tests, to be able to mock API commands. If nil, will use
+	// bitlocker.GetEncryptionStatus
+	execGetEncryptionStatusFn execGetEncryptionStatusFunc
 }
 
 func ApplyWindowsMDMBitlockerFetcherMiddleware(
@@ -457,41 +472,125 @@ func (w *windowsMDMBitlockerConfigFetcher) GetConfig() (*fleet.OrbitConfig, erro
 }
 
 func (w *windowsMDMBitlockerConfigFetcher) attemptBitlockerEncryption(notifs fleet.OrbitConfigNotifications) {
-	// do not trigger Bitlocker encryption if running on a Windwos server
+	if time.Since(w.lastRun) <= w.Frequency {
-	isWindowsServer, err := IsRunningOnWindowsServer()
-	if err != nil {
-		log.Error().Err(err).Msg("checking if the host is a Windows server")
-		return
-	}
-
-	if isWindowsServer {
-		log.Debug().Msg("device is a Windows Server, encryption is not going to be performed")
-		return
-	}
-
-	if time.Since(w.lastEnrollRun) <= w.Frequency {
 		log.Debug().Msg("skipped encryption process, last run was too recent")
 		return
 	}
 
-	// Performing Bitlocker encryption operation against C: volume
+	// Windows servers are not supported. Check and skip if that's the case.
+	if isServer, err := IsRunningOnWindowsServer(); isServer || err != nil {
+		if err != nil {
+			log.Error().Err(err).Msg("checking if the host is a Windows server")
+		} else {
+			log.Debug().Msg("device is a Windows Server, encryption is not going to be performed")
+		}
+		return
+	}
 
-	// We are supporting only C: volume for now
+	const targetVolume = "C:"
-	targetVolume := "C:"
+	encryptionStatus, err := w.getEncryptionStatusForVolume(targetVolume)
+	if err != nil {
+		log.Debug().Err(err).Msgf("unable to get encryption status for target volume %s, continuing anyway", targetVolume)
+	}
 
-	// Performing actual encryption
+	// don't do anything if the disk is being encrypted/decrypted
+	if w.bitLockerActionInProgress(encryptionStatus) {
+		log.Debug().Msgf("skipping encryption as the disk is not available. Disk conversion status: %d", encryptionStatus.ConversionStatus)
+		return
+	}
 
-	// Getting Bitlocker encryption mock operation function if any
+	recoveryKey, encryptionErr := w.performEncryption(targetVolume)
+	// before reporting the error to the server, check if the error we've got is valid.
+	// see the description of w.isMisreportedDecryptionError and issue #15916.
+	var pErr *bitlocker.EncryptionError
+	if errors.As(encryptionErr, &pErr) && w.isMisreportedDecryptionError(pErr, encryptionStatus) {
+		log.Error().Msg("disk encryption failed due to previous unsuccessful attempt, user action required")
+		return
+	}
+
+	if serverErr := w.updateFleetServer(recoveryKey, encryptionErr); serverErr != nil {
+		log.Error().Err(serverErr).Msg("failed to send encryption result to Fleet Server")
+		return
+	}
+
+	if encryptionErr != nil {
+		log.Error().Err(err).Msg("failed to encrypt the volume")
+		return
+	}
+
+	w.lastRun = time.Now()
+}
+
+// getEncryptionStatusForVolume retrieves the encryption status for a specific volume.
+func (w *windowsMDMBitlockerConfigFetcher) getEncryptionStatusForVolume(volume string) (*bitlocker.EncryptionStatus, error) {
+	fn := w.execGetEncryptionStatusFn
+	if fn == nil {
+		fn = bitlocker.GetEncryptionStatus
+	}
+	status, err := fn()
+	if err != nil {
+		return nil, err
+	}
+
+	for _, s := range status {
+		if s.DriveVolume == volume {
+			return s.Status, nil
+		}
+	}
+
+	return nil, nil
+}
+
+// bitLockerActionInProgress determines an encryption/decription action is in
+// progress based on the reported status.
+func (w *windowsMDMBitlockerConfigFetcher) bitLockerActionInProgress(status *bitlocker.EncryptionStatus) bool {
+	if status == nil {
+		return false
+	}
+
+	// Check if the status matches any of the specified conditions
+	return status.ConversionStatus == bitlocker.ConversionStatusDecryptionInProgress ||
+		status.ConversionStatus == bitlocker.ConversionStatusDecryptionPaused ||
+		status.ConversionStatus == bitlocker.ConversionStatusEncryptionInProgress ||
+		status.ConversionStatus == bitlocker.ConversionStatusEncryptionPaused
+}
+
+// performEncryption executes the encryption process.
+func (w *windowsMDMBitlockerConfigFetcher) performEncryption(volume string) (string, error) {
 	fn := w.execEncryptVolumeFn
 	if fn == nil {
-		// Otherwise, using the real one
 		fn = bitlocker.EncryptVolume
 	}
 
-	// Encryption operation is performed here, err will be captured if any
+	recoveryKey, err := fn(volume)
-	// Error will be returned if the encryption operation failed after sending it to Fleet Server
+	if err != nil {
-	recoveryKey, err := fn(targetVolume)
+		return "", err
+	}
 
+	return recoveryKey, nil
+}
+
+// isMisreportedDecryptionError checks whether the given error is a potentially
+// misreported decryption error.
+//
+// It addresses cases where a previous encryption attempt failed due to other
+// errors but subsequent attempts to encrypt the disk could erroneously return
+// a bitlocker.FVE_E_NOT_DECRYPTED error.
+//
+// This function checks if the disk is actually fully decrypted
+// (status.ConversionStatus == bitlocker.CONVERSION_STATUS_FULLY_DECRYPTED) and
+// whether the reported error is bitlocker.FVE_E_NOT_DECRYPTED. If these
+// conditions are met, the error is not accurately reflecting the disk's actual
+// encryption state.
+//
+// For more context, see issue #15916
+func (w *windowsMDMBitlockerConfigFetcher) isMisreportedDecryptionError(err *bitlocker.EncryptionError, status *bitlocker.EncryptionStatus) bool {
+	return err.Code() == bitlocker.ErrorCodeNotDecrypted &&
+		status != nil &&
+		status.ConversionStatus == bitlocker.ConversionStatusFullyDecrypted
+}
+
+func (w *windowsMDMBitlockerConfigFetcher) updateFleetServer(key string, err error) error {
 	// Getting Bitlocker encryption operation error message if any
 	// This is going to be sent to Fleet Server
 	bitlockerError := ""
@@ -501,22 +600,9 @@ func (w *windowsMDMBitlockerConfigFetcher) attemptBitlockerEncryption(notifs fle
 
 	// Update Fleet Server with encryption result
 	payload := fleet.OrbitHostDiskEncryptionKeyPayload{
-		EncryptionKey: []byte(recoveryKey),
+		EncryptionKey: []byte(key),
 		ClientError:   bitlockerError,
 	}
 
-	errServerUpdate := w.EncryptionResult.SetOrUpdateDiskEncryptionKey(payload)
+	return w.EncryptionResult.SetOrUpdateDiskEncryptionKey(payload)
-	if errServerUpdate != nil {
-		log.Error().Err(errServerUpdate).Msg("failed to send encryption result to Fleet Server")
-		return
-	}
-
-	// This is the error status of the Bitlocker encryption operation
-	// it is returned here after sending the result to Fleet Server
-	if err != nil {
-		log.Error().Err(err).Msg("failed to encrypt the volume")
-		return
-	}
-
-	w.lastEnrollRun = time.Now()
 }

orbit/pkg/update/notifications_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/fleetdm/fleet/v4/orbit/pkg/bitlocker"
 	"github.com/fleetdm/fleet/v4/orbit/pkg/scripts"
 	"github.com/fleetdm/fleet/v4/server/fleet"
 	"github.com/fleetdm/fleet/v4/server/ptr"
@@ -574,10 +575,14 @@ func TestRunScripts(t *testing.T) {
 	})
 }
 
-type mockDiskEncryptionKeySetter struct{}
+type mockDiskEncryptionKeySetter struct {
+	SetOrUpdateDiskEncryptionKeyImpl    func(diskEncryptionStatus fleet.OrbitHostDiskEncryptionKeyPayload) error
+	SetOrUpdateDiskEncryptionKeyInvoked bool
+}
 
-func (m mockDiskEncryptionKeySetter) SetOrUpdateDiskEncryptionKey(diskEncryptionStatus fleet.OrbitHostDiskEncryptionKeyPayload) error {
+func (m *mockDiskEncryptionKeySetter) SetOrUpdateDiskEncryptionKey(diskEncryptionStatus fleet.OrbitHostDiskEncryptionKeyPayload) error {
-	return nil
+	m.SetOrUpdateDiskEncryptionKeyInvoked = true
+	return m.SetOrUpdateDiskEncryptionKeyImpl(diskEncryptionStatus)
 }
 
 func TestBitlockerOperations(t *testing.T) {
@@ -588,8 +593,9 @@ func TestBitlockerOperations(t *testing.T) {
 	t.Cleanup(func() { log.Logger = oldLog })
 
 	var (
-		shouldEncrypt     = true
+		shouldEncrypt          = true
-		shouldReturnError = false
+		shouldFailEncryption   = false
+		shouldFailServerUpdate = false
 	)
 
 	fetcher := &dummyConfigFetcher{
@@ -600,40 +606,142 @@ func TestBitlockerOperations(t *testing.T) {
 		},
 	}
 
-	enrollFetcher := &windowsMDMBitlockerConfigFetcher{
+	clientMock := &mockDiskEncryptionKeySetter{}
-		Fetcher:          fetcher,
+	clientMock.SetOrUpdateDiskEncryptionKeyImpl = func(diskEncryptionStatus fleet.OrbitHostDiskEncryptionKeyPayload) error {
-		Frequency:        time.Hour, // doesn't matter for this test
+		if shouldFailServerUpdate {
-		EncryptionResult: mockDiskEncryptionKeySetter{},
+			return errors.New("server error")
-		execEncryptVolumeFn: func(string) (string, error) {
+		}
-			if shouldReturnError {
+		return nil
-				return "", errors.New("error")
+	}
-			}
 
-			return "123456", nil
+	var enrollFetcher *windowsMDMBitlockerConfigFetcher
-		},
+	setupTest := func() {
+		enrollFetcher = &windowsMDMBitlockerConfigFetcher{
+			Fetcher:          fetcher,
+			Frequency:        time.Hour, // doesn't matter for this test
+			lastRun:          time.Now().Add(-2 * time.Hour),
+			EncryptionResult: clientMock,
+			execGetEncryptionStatusFn: func() ([]bitlocker.VolumeStatus, error) {
+				return []bitlocker.VolumeStatus{}, nil
+			},
+			execEncryptVolumeFn: func(string) (string, error) {
+				if shouldFailEncryption {
+					return "", errors.New("error")
+				}
+
+				return "123456", nil
+			},
+		}
+		clientMock.SetOrUpdateDiskEncryptionKeyInvoked = false
+		logBuf.Reset()
 	}
 
 	t.Run("bitlocker encryption is performed", func(t *testing.T) {
+		setupTest()
 		shouldEncrypt = true
-		shouldReturnError = false
+		shouldFailEncryption = false
 		cfg, err := enrollFetcher.GetConfig()
 		require.NoError(t, err)            // the dummy fetcher never returns an error
 		require.Equal(t, fetcher.cfg, cfg) // the bitlocker wrapper properly returns the expected config
 	})
 
 	t.Run("bitlocker encryption is not performed", func(t *testing.T) {
+		setupTest()
 		shouldEncrypt = false
-		shouldReturnError = false
+		shouldFailEncryption = false
 		cfg, err := enrollFetcher.GetConfig()
 		require.NoError(t, err)            // the dummy fetcher never returns an error
 		require.Equal(t, fetcher.cfg, cfg) // the bitlocker wrapper properly returns the expected config
 	})
 
 	t.Run("bitlocker encryption returns an error", func(t *testing.T) {
+		setupTest()
 		shouldEncrypt = true
-		shouldReturnError = true
+		shouldFailEncryption = true
 		cfg, err := enrollFetcher.GetConfig()
 		require.NoError(t, err)            // the dummy fetcher never returns an error
 		require.Equal(t, fetcher.cfg, cfg) // the bitlocker wrapper properly returns the expected config
 	})
+
+	t.Run("encryption skipped based on various current statuses", func(t *testing.T) {
+		setupTest()
+		statusesToTest := []int32{
+			bitlocker.ConversionStatusDecryptionInProgress,
+			bitlocker.ConversionStatusDecryptionPaused,
+			bitlocker.ConversionStatusEncryptionInProgress,
+			bitlocker.ConversionStatusEncryptionPaused,
+		}
+
+		for _, status := range statusesToTest {
+			t.Run(fmt.Sprintf("status %d", status), func(t *testing.T) {
+				mockStatus := &bitlocker.EncryptionStatus{ConversionStatus: status}
+				enrollFetcher.execGetEncryptionStatusFn = func() ([]bitlocker.VolumeStatus, error) {
+					return []bitlocker.VolumeStatus{{DriveVolume: "C:", Status: mockStatus}}, nil
+				}
+
+				cfg, err := enrollFetcher.GetConfig()
+				require.NoError(t, err)
+				require.Equal(t, fetcher.cfg, cfg)
+				require.Contains(t, logBuf.String(), "skipping encryption as the disk is not available")
+				logBuf.Reset() // Reset the log buffer for the next iteration
+			})
+		}
+	})
+
+	t.Run("handle misreported decryption error", func(t *testing.T) {
+		setupTest()
+		mockStatus := &bitlocker.EncryptionStatus{ConversionStatus: bitlocker.ConversionStatusFullyDecrypted}
+		enrollFetcher.execGetEncryptionStatusFn = func() ([]bitlocker.VolumeStatus, error) {
+			return []bitlocker.VolumeStatus{{DriveVolume: "C:", Status: mockStatus}}, nil
+		}
+		enrollFetcher.execEncryptVolumeFn = func(string) (string, error) {
+			return "", bitlocker.NewEncryptionError("", bitlocker.ErrorCodeNotDecrypted)
+		}
+
+		cfg, err := enrollFetcher.GetConfig()
+		require.NoError(t, err)
+		require.Equal(t, fetcher.cfg, cfg)
+		require.Contains(t, logBuf.String(), "disk encryption failed due to previous unsuccessful attempt")
+	})
+
+	t.Run("encryption skipped if last run too recent", func(t *testing.T) {
+		setupTest()
+		enrollFetcher.lastRun = time.Now().Add(-30 * time.Minute)
+		enrollFetcher.Frequency = 1 * time.Hour
+
+		cfg, err := enrollFetcher.GetConfig()
+		require.NoError(t, err)
+		require.Equal(t, fetcher.cfg, cfg)
+		require.Contains(t, logBuf.String(), "skipped encryption process, last run was too recent")
+	})
+
+	t.Run("successful fleet server update", func(t *testing.T) {
+		setupTest()
+		shouldFailEncryption = false
+		mockStatus := &bitlocker.EncryptionStatus{ConversionStatus: bitlocker.ConversionStatusFullyDecrypted}
+		enrollFetcher.execGetEncryptionStatusFn = func() ([]bitlocker.VolumeStatus, error) {
+			return []bitlocker.VolumeStatus{{DriveVolume: "C:", Status: mockStatus}}, nil
+		}
+
+		cfg, err := enrollFetcher.GetConfig()
+		require.NoError(t, err)
+		require.Equal(t, fetcher.cfg, cfg)
+		require.True(t, clientMock.SetOrUpdateDiskEncryptionKeyInvoked)
+	})
+
+	t.Run("failed fleet server update", func(t *testing.T) {
+		setupTest()
+		shouldFailEncryption = false
+		shouldFailServerUpdate = true
+		mockStatus := &bitlocker.EncryptionStatus{ConversionStatus: bitlocker.ConversionStatusFullyDecrypted}
+		enrollFetcher.execGetEncryptionStatusFn = func() ([]bitlocker.VolumeStatus, error) {
+			return []bitlocker.VolumeStatus{{DriveVolume: "C:", Status: mockStatus}}, nil
+		}
+
+		cfg, err := enrollFetcher.GetConfig()
+		require.NoError(t, err)
+		require.Equal(t, fetcher.cfg, cfg)
+		require.Contains(t, logBuf.String(), "failed to send encryption result to Fleet Server")
+		require.True(t, clientMock.SetOrUpdateDiskEncryptionKeyInvoked)
+	})
 }