-
Notifications
You must be signed in to change notification settings - Fork 843
Bitlocker: do not decrypt already encrypted drive. #43130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
99b0d56
d9daa34
69b66d0
62aeec3
060ee29
e2d79b4
faf2081
261b740
f35ec5f
373b1fc
1da1f70
a762ebe
5b34ad9
3c559e1
e9e35b4
31978c8
bab1468
a0484d8
d745a68
582dd7e
363a3ac
b0c6c42
6739e07
7590bff
30b231b
500183a
9eb77c1
3e5e474
ff5ece4
dfae539
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - Fixed a Windows BitLocker encrypt/decrypt loop on machines with secondary drives using auto-unlock. Fleet now detects disk encryption using `conversion_status` (not just `protection_status`), preventing the server from repeatedly requesting encryption when the disk is already encrypted. Added `bitlocker_protection_status` tracking so the UI shows "Action required" when BitLocker protection is off instead of misleadingly showing "Verified." |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - When a Windows disk is already encrypted and Fleet needs the recovery key, orbit now rotates the recovery key (adds a new Fleet-managed protector and removes old ones) instead of decrypting and re-encrypting the entire disk. This avoids the FVE_E_AUTOUNLOCK_ENABLED error loop on machines with secondary drives using auto-unlock. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,6 +65,14 @@ const ( | |
| EncryptionTypeHardware ForceEncryptionType = 2 | ||
| ) | ||
|
|
||
| // fveErrorCode formats a BitLocker error code as "unsigned_decimal (0xHEX)" to | ||
| // match the format used in the Microsoft WMI documentation, making errors | ||
| // searchable. The WMI docs define return values as uint32 but the COM VARIANT | ||
| // transport delivers them as int32 (see comment on the error code constants). | ||
| func fveErrorCode(val int32) string { | ||
| return fmt.Sprintf("%d (0x%08x)", uint32(val), uint32(val)) // nolint:gosec | ||
| } | ||
|
|
||
| func encryptErrHandler(val int32) error { | ||
| var msg string | ||
|
|
||
|
|
@@ -90,7 +98,7 @@ func encryptErrHandler(val int32) error { | |
| case ErrorCodeProtectorExists: | ||
| msg = "key protector cannot be added; only one key protector of this type is allowed for this drive" | ||
| default: | ||
| msg = fmt.Sprintf("error code returned during encryption: %d", val) | ||
| msg = fmt.Sprintf("error code returned during encryption: %s", fveErrorCode(val)) | ||
| } | ||
|
|
||
| return &EncryptionError{msg, val} | ||
|
|
@@ -137,20 +145,6 @@ func (v *Volume) encrypt(method EncryptionMethod, flags EncryptionFlag) error { | |
| return nil | ||
| } | ||
|
|
||
| // decrypt encrypts the volume | ||
| // Example: vol.decrypt() | ||
| // https://learn.microsoft.com/en-us/windows/win32/secprov/decrypt-win32-encryptablevolume | ||
| func (v *Volume) decrypt() error { | ||
| resultRaw, err := oleutil.CallMethod(v.handle, "Decrypt") | ||
| if err != nil { | ||
| return fmt.Errorf("decrypt(%s): %w", v.letter, err) | ||
| } else if val, ok := resultRaw.Value().(int32); val != 0 || !ok { | ||
| return fmt.Errorf("decrypt(%s): %w", v.letter, encryptErrHandler(val)) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // prepareVolume prepares a new Bitlocker Volume. This should be called BEFORE any key protectors are added. | ||
| // Example: vol.prepareVolume(bitlocker.VolumeTypeDefault, bitlocker.EncryptionTypeHardware) | ||
| // https://docs.microsoft.com/en-us/windows/win32/secprov/preparevolume-win32-encryptablevolume | ||
|
|
@@ -227,6 +221,61 @@ func (v *Volume) deleteKeyProtectors() error { | |
| return nil | ||
| } | ||
|
|
||
| // deleteKeyProtector removes a single key protector by its ID. | ||
| // https://learn.microsoft.com/en-us/windows/win32/secprov/deletekeyprotector-win32-encryptablevolume | ||
| func (v *Volume) deleteKeyProtector(protectorID string) error { | ||
| resultRaw, err := oleutil.CallMethod(v.handle, "DeleteKeyProtector", protectorID) | ||
| if err != nil { | ||
| return fmt.Errorf("deleteKeyProtector(%s, %s): %w", v.letter, protectorID, err) | ||
| } | ||
| if val, ok := resultRaw.Value().(int32); val != 0 || !ok { | ||
| return fmt.Errorf("deleteKeyProtector(%s, %s): %w", v.letter, protectorID, encryptErrHandler(val)) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // Key protector types for GetKeyProtectors. | ||
| // https://learn.microsoft.com/en-us/windows/win32/secprov/getkeyprotectors-win32-encryptablevolume | ||
| const ( | ||
| KeyProtectorTypeNumericalPassword int32 = 3 | ||
| ) | ||
|
|
||
| // getKeyProtectorIDs returns the IDs of key protectors of the given type. | ||
| // https://learn.microsoft.com/en-us/windows/win32/secprov/getkeyprotectors-win32-encryptablevolume | ||
| func (v *Volume) getKeyProtectorIDs(protectorType int32) ([]string, error) { | ||
| var protectorIDs ole.VARIANT | ||
| _ = ole.VariantInit(&protectorIDs) | ||
| defer ole.VariantClear(&protectorIDs) //nolint:errcheck | ||
|
|
||
| resultRaw, err := oleutil.CallMethod(v.handle, "GetKeyProtectors", protectorType, &protectorIDs) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("getKeyProtectors(%s, %d): %w", v.letter, protectorType, err) | ||
| } | ||
| if val, ok := resultRaw.Value().(int32); val != 0 || !ok { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like the docs (https://learn.microsoft.com/en-us/windows/win32/secprov/getkeyprotectors-win32-encryptablevolume) state that
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I looked into this. It looks like the library we're using I had Claude do a test case on a Windows machine to verify the resultRaw type. So since we get int32, we check for errors in that type, like: I'll file a bug or fix it in a separate PR since we return these values to the user.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I fixed the error message to not return negative numbers. Now it is: |
||
| return nil, fmt.Errorf("getKeyProtectors(%s, %d): %w", v.letter, protectorType, encryptErrHandler(val)) | ||
| } | ||
|
|
||
| // The WMI method returns an out-parameter VARIANT containing a SAFEARRAY. | ||
| // The array type is VT_ARRAY|VT_VARIANT (0x200C), not VT_ARRAY|VT_BSTR. | ||
| // We use ToValueArray() to extract each element as an interface{}, then | ||
| // convert to strings. We do NOT call safeArray.Release() here because | ||
| // ToArray() wraps the same pointer from the VARIANT without copying -- | ||
| // defer VariantClear above handles freeing the SAFEARRAY. | ||
| safeArray := protectorIDs.ToArray() | ||
| if safeArray == nil { | ||
| return nil, nil | ||
| } | ||
|
|
||
| values := safeArray.ToValueArray() | ||
| result := make([]string, 0, len(values)) | ||
| for _, v := range values { | ||
| if s, ok := v.(string); ok { | ||
| result = append(result, s) | ||
|
getvictor marked this conversation as resolved.
|
||
| } | ||
| } | ||
| return result, nil | ||
| } | ||
|
getvictor marked this conversation as resolved.
getvictor marked this conversation as resolved.
|
||
|
|
||
| // getBitlockerStatus returns the current status of the volume | ||
| // https://learn.microsoft.com/en-us/windows/win32/secprov/getprotectionstatus-win32-encryptablevolume | ||
| func (v *Volume) getBitlockerStatus() (*EncryptionStatus, error) { | ||
|
|
@@ -484,20 +533,45 @@ func encryptVolumeOnCOMThread(targetVolume string) (string, error) { | |
| return recoveryKey, nil | ||
| } | ||
|
|
||
| func decryptVolumeOnCOMThread(targetVolume string) error { | ||
| // Connect to the volume | ||
| // rotateRecoveryKeyOnCOMThread rotates the recovery key on an already-encrypted volume. | ||
| // It adds a new Fleet-managed recovery key protector, removes old recovery key protectors, | ||
| // and returns the new recovery key for escrow. The disk is never decrypted. | ||
| func rotateRecoveryKeyOnCOMThread(targetVolume string) (string, error) { | ||
| vol, err := bitlockerConnect(targetVolume) | ||
| if err != nil { | ||
| return fmt.Errorf("connecting to the volume: %w", err) | ||
| return "", fmt.Errorf("connecting to the volume: %w", err) | ||
| } | ||
| defer vol.bitlockerClose() | ||
|
|
||
| // Start decryption | ||
| if err := vol.decrypt(); err != nil { | ||
| return fmt.Errorf("starting decryption: %w", err) | ||
| // Get existing numerical password (recovery key) protector IDs before adding a new one. | ||
| oldProtectorIDs, err := vol.getKeyProtectorIDs(KeyProtectorTypeNumericalPassword) | ||
| if err != nil { | ||
| return "", fmt.Errorf("listing existing recovery key protectors: %w", err) | ||
| } | ||
|
|
||
| return nil | ||
| // Add a new recovery key protector. Windows generates the recovery password. | ||
| newRecoveryKey, err := vol.protectWithNumericalPassword() | ||
| if err != nil { | ||
| return "", fmt.Errorf("adding new recovery key protector: %w", err) | ||
| } | ||
|
|
||
| // Remove old recovery key protectors so previously compromised keys are invalidated. | ||
| for _, oldID := range oldProtectorIDs { | ||
| if err := vol.deleteKeyProtector(oldID); err != nil { | ||
| log.Warn().Err(err).Str("protector_id", oldID).Msg("could not delete old recovery key protector, continuing") | ||
| } | ||
| } | ||
|
|
||
| // Ensure a TPM protector exists (some pre-encrypted disks may not have one). | ||
| if err := vol.protectWithTPM(nil); err != nil { | ||
| // ErrorCodeProtectorExists is expected if a TPM protector is already present. | ||
| var encErr *EncryptionError | ||
| if !errors.As(err, &encErr) || encErr.Code() != ErrorCodeProtectorExists { | ||
| log.Debug().Err(err).Msg("could not add TPM protector, continuing") | ||
| } | ||
| } | ||
|
|
||
| return newRecoveryKey, nil | ||
| } | ||
|
|
||
| func getEncryptionStatusOnCOMThread() ([]VolumeStatus, error) { | ||
|
getvictor marked this conversation as resolved.
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here. The spec seems to say this method returns a
uint32There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above