attempt to decrypt the disk before performing a BitLocker encryption (#16097)

for #15711, this attempts to decrypt the disk if it was previously
encrypted and Fleet doesn't have the key.
This commit is contained in:
Roberto Dip 2024-01-16 12:45:23 -03:00 committed by GitHub
parent 18673e8bc6
commit fedeab6130
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 111 additions and 2 deletions

View file

@ -0,0 +1 @@
* Attempt to automatically decrypt the disk before performing a BitLocker encryption if it was previously encrypted and Fleet doesn't have the key.

View file

@ -416,6 +416,12 @@ type execEncryptVolumeFunc func(volumeID string) (recoveryKey string, err error)
// encryption status of a volume, and an error if the operation fails.
type execGetEncryptionStatusFunc func() (status []bitlocker.VolumeStatus, err error)
// execDecryptVolumeFunc handles the decryption of a volume identified by its
// string identifier (e.g., "C:")
//
// It returns an error if the process fails.
type execDecryptVolumeFunc func(volumeID string) error
type windowsMDMBitlockerConfigFetcher struct {
// Fetcher is the OrbitConfigFetcher that will be wrapped. It is responsible
// for actually returning the orbit configuration or an error.
@ -441,6 +447,10 @@ type windowsMDMBitlockerConfigFetcher struct {
// for tests, to be able to mock API commands. If nil, will use
// bitlocker.GetEncryptionStatus
execGetEncryptionStatusFn execGetEncryptionStatusFunc
// for tests, to be able to mock the decryption process. If nil, will use
// bitlocker.DecryptVolume
execDecryptVolumeFn execDecryptVolumeFunc
}
func ApplyWindowsMDMBitlockerFetcherMiddleware(
@ -499,6 +509,29 @@ func (w *windowsMDMBitlockerConfigFetcher) attemptBitlockerEncryption(notifs fle
return
}
// if the disk is encrypted, try to decrypt it first.
if encryptionStatus != nil &&
encryptionStatus.ConversionStatus == bitlocker.ConversionStatusFullyEncrypted {
log.Debug().Msg("disk was previously encrypted. Attempting to decrypt it")
if err := w.decryptVolume(targetVolume); err != nil {
log.Error().Err(err).Msg("decryption failed")
if serverErr := w.updateFleetServer("", err); serverErr != nil {
log.Error().Err(serverErr).Msg("failed to send decryption failure to Fleet Server")
return
}
}
// return regardless of the operation output.
//
// the decryption process takes an unknown amount of time (depending on
// factors outside of our control) and the next tick will be a noop if the
// disk is not ready to be encrypted yet (due to the
// w.bitLockerActionInProgress check above)
return
}
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.
@ -570,6 +603,15 @@ func (w *windowsMDMBitlockerConfigFetcher) performEncryption(volume string) (str
return recoveryKey, nil
}
func (w *windowsMDMBitlockerConfigFetcher) decryptVolume(targetVolume string) error {
fn := w.execDecryptVolumeFn
if fn == nil {
fn = bitlocker.DecryptVolume
}
return fn(targetVolume)
}
// isMisreportedDecryptionError checks whether the given error is a potentially
// misreported decryption error.
//

View file

@ -595,7 +595,10 @@ func TestBitlockerOperations(t *testing.T) {
var (
shouldEncrypt = true
shouldFailEncryption = false
shouldFailDecryption = false
shouldFailServerUpdate = false
encryptFnCalled = false
decryptFnCalled = false
)
fetcher := &dummyConfigFetcher{
@ -625,13 +628,28 @@ func TestBitlockerOperations(t *testing.T) {
return []bitlocker.VolumeStatus{}, nil
},
execEncryptVolumeFn: func(string) (string, error) {
encryptFnCalled = true
if shouldFailEncryption {
return "", errors.New("error")
return "", errors.New("error encrypting")
}
return "123456", nil
},
execDecryptVolumeFn: func(string) error {
decryptFnCalled = true
if shouldFailDecryption {
return errors.New("error decrypting")
}
return nil
},
}
shouldEncrypt = true
shouldFailEncryption = false
shouldFailDecryption = false
shouldFailServerUpdate = false
encryptFnCalled = false
decryptFnCalled = false
clientMock.SetOrUpdateDiskEncryptionKeyInvoked = false
logBuf.Reset()
}
@ -640,6 +658,7 @@ func TestBitlockerOperations(t *testing.T) {
setupTest()
shouldEncrypt = true
shouldFailEncryption = false
shouldFailDecryption = 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
@ -652,6 +671,8 @@ func TestBitlockerOperations(t *testing.T) {
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
require.True(t, encryptFnCalled, "encryption function should have been called")
require.False(t, decryptFnCalled, "decryption function should not be called")
})
t.Run("bitlocker encryption returns an error", func(t *testing.T) {
@ -661,6 +682,8 @@ func TestBitlockerOperations(t *testing.T) {
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
require.True(t, encryptFnCalled, "encryption function should have been called")
require.False(t, decryptFnCalled, "decryption function should not be called")
})
t.Run("encryption skipped based on various current statuses", func(t *testing.T) {
@ -683,6 +706,8 @@ func TestBitlockerOperations(t *testing.T) {
require.NoError(t, err)
require.Equal(t, fetcher.cfg, cfg)
require.Contains(t, logBuf.String(), "skipping encryption as the disk is not available")
require.False(t, encryptFnCalled, "encryption function should not be called")
require.False(t, decryptFnCalled, "decryption function should not be called")
logBuf.Reset() // Reset the log buffer for the next iteration
})
}
@ -701,7 +726,42 @@ func TestBitlockerOperations(t *testing.T) {
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")
require.Contains(t, logBuf.String(), "disk encryption failed due to previous unsuccessful attempt, user action required")
require.False(t, encryptFnCalled, "encryption function should not be called")
require.False(t, decryptFnCalled, "decryption function should not be called")
})
t.Run("decrypts the disk if previously encrypted", func(t *testing.T) {
setupTest()
mockStatus := &bitlocker.EncryptionStatus{ConversionStatus: bitlocker.ConversionStatusFullyEncrypted}
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(), "disk was previously encrypted. Attempting to decrypt it")
require.False(t, clientMock.SetOrUpdateDiskEncryptionKeyInvoked)
require.False(t, encryptFnCalled, "encryption function should not have been called")
require.True(t, decryptFnCalled, "decryption function should have been called")
})
t.Run("reports to the server if decryption fails", func(t *testing.T) {
setupTest()
shouldFailDecryption = true
mockStatus := &bitlocker.EncryptionStatus{ConversionStatus: bitlocker.ConversionStatusFullyEncrypted}
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(), "disk was previously encrypted. Attempting to decrypt it")
require.Contains(t, logBuf.String(), "decryption failed")
require.True(t, clientMock.SetOrUpdateDiskEncryptionKeyInvoked)
require.False(t, encryptFnCalled, "encryption function should not be called")
require.True(t, decryptFnCalled, "decryption function should have been called")
})
t.Run("encryption skipped if last run too recent", func(t *testing.T) {
@ -713,6 +773,8 @@ func TestBitlockerOperations(t *testing.T) {
require.NoError(t, err)
require.Equal(t, fetcher.cfg, cfg)
require.Contains(t, logBuf.String(), "skipped encryption process, last run was too recent")
require.False(t, encryptFnCalled, "encryption function should not be called")
require.False(t, decryptFnCalled, "decryption function should not be called")
})
t.Run("successful fleet server update", func(t *testing.T) {
@ -727,6 +789,8 @@ func TestBitlockerOperations(t *testing.T) {
require.NoError(t, err)
require.Equal(t, fetcher.cfg, cfg)
require.True(t, clientMock.SetOrUpdateDiskEncryptionKeyInvoked)
require.True(t, encryptFnCalled, "encryption function should have been called")
require.False(t, decryptFnCalled, "decryption function should not be called")
})
t.Run("failed fleet server update", func(t *testing.T) {
@ -743,5 +807,7 @@ func TestBitlockerOperations(t *testing.T) {
require.Equal(t, fetcher.cfg, cfg)
require.Contains(t, logBuf.String(), "failed to send encryption result to Fleet Server")
require.True(t, clientMock.SetOrUpdateDiskEncryptionKeyInvoked)
require.True(t, encryptFnCalled, "encryption function should have been called")
require.False(t, decryptFnCalled, "decryption function should not be called")
})
}