From 663190abb66fde1d9a9d386e689c7c7141a9e969 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Wed, 3 Jun 2026 11:32:00 +0000 Subject: [PATCH 1/5] Unlock Bitlocker encrypted volumes Just like in case of LUKS (https://github.com/cloudbase/coriolis/pull/436), we'll let Coriolis users specify a BitLocker recovery password. At the very least it should unlock the OS volume, however it may be used for other encrypted volumes as well. If no encrypted volume could be unlocked using the specified key, Coriolis will error out. In addition to that, we'll temporarily suspend BitLocker on the specified volumes. It won't decrypt the volumes, it merely adds a publicly accessible protector that allows the replica instance to boot. Once the replica instance boots, BitLocker will be resumed automatically and the TPM protector will be reconfigured. --- coriolis/osmorphing/osmount/windows.py | 65 ++++++++ .../tests/osmorphing/osmount/test_windows.py | 145 ++++++++++++++++++ coriolis/tests/osmorphing/test_manager.py | 1 + 3 files changed, 211 insertions(+) diff --git a/coriolis/osmorphing/osmount/windows.py b/coriolis/osmorphing/osmount/windows.py index 2d8236e2a..dd9c1ca2e 100644 --- a/coriolis/osmorphing/osmount/windows.py +++ b/coriolis/osmorphing/osmount/windows.py @@ -6,6 +6,7 @@ from oslo_log import log as logging +from coriolis import constants from coriolis import exception from coriolis.osmorphing.osmount import base from coriolis import utils @@ -223,9 +224,73 @@ def _set_volumes_drive_letter(self): f"Error was: {utils.get_exception_details()}") self._rebring_disks_online(disk_nums=disk_nums) + def _get_encrypted_volume_ids(self): + out = self._conn.exec_ps_command( + 'gwmi -ns "Root\\CIMV2\\Security\\MicrosoftVolumeEncryption" ' + '-class Win32_EncryptableVolume | % {$_.DeviceID}') + return [x for x in out.replace("\r\n", "\n").split("\n") if x] + + def _unlock_encrypted_volume(self, volume_id: str, recovery_password: str): + self._conn.exec_ps_command( + f'manage-bde -unlock "{volume_id}" ' + f'-RecoveryPassword "{recovery_password}"') + + def _suspend_bitlocker(self, volume_id: str): + """Suspend BitLocker until the next reboot for a given volume. + + It doesn't decrypt the device, it just adds a publicly accessible + BitLocker protector that automatically unlocks the volume. + + When the replica instance boots, BitLocker will be automatically + enabled again and the TPM protector reconfigured. + """ + self._conn.exec_ps_command(f'Suspend-BitLocker "{volume_id}"') + + def _unlock_encrypted_volumes(self): + recovery_password = self._osmorphing_info.get( + constants.ENCRYPTED_DISKS_PASS) + if not recovery_password: + LOG.info("No encrypted disk password specified, " + "skipping BitLocker unlock.") + return + + encrypted_volume_ids = self._get_encrypted_volume_ids() + if not encrypted_volume_ids: + LOG.warning("Received encrypted disk password but no " + "BitLocker encrypted volumes found.") + return + + unlocked = False + for encrypted_volume_id in encrypted_volume_ids: + try: + self._unlock_encrypted_volume( + encrypted_volume_id, recovery_password) + LOG.info( + "Successfully unlocked BitLocker encrypted volume: %s", + encrypted_volume_id) + unlocked = True + except Exception: + LOG.info( + "Could not unlock volume %s using the specified " + "recovery password.", + encrypted_volume_id) + continue + + # Suspend BitLocker until the replica boots. + # + # We'll intentionally propagate the failure if we managed to + # unlock the volume but failed to suspend BitLocker. + self._suspend_bitlocker(encrypted_volume_id) + + if not unlocked: + raise exception.CoriolisException( + "Could not unlock any volume using the specified " + "BitLocker recovery password.") + def mount_os(self): self._set_basic_disks_rw_mode() self._bring_disks_online() + self._unlock_encrypted_volumes() self._set_volumes_drive_letter() fs_roots = utils.retry_on_error(sleep_seconds=5)(self._get_fs_roots)( fail_if_empty=True) diff --git a/coriolis/tests/osmorphing/osmount/test_windows.py b/coriolis/tests/osmorphing/osmount/test_windows.py index 56dd83435..b9eab554e 100644 --- a/coriolis/tests/osmorphing/osmount/test_windows.py +++ b/coriolis/tests/osmorphing/osmount/test_windows.py @@ -4,6 +4,7 @@ import logging from unittest import mock +from coriolis import constants from coriolis import exception from coriolis.osmorphing.osmount import windows from coriolis.tests import test_base @@ -336,3 +337,147 @@ def test_dismount_os(self): self.tools._conn.exec_ps_command.assert_called_once_with( '(Get-Disk | Where-Object { $_.IsBoot -eq $False }).Number') + + def test_get_encrypted_volume_ids(self): + # Powershell wouldn't mix line endings, we're just ensuring that + # we can properly handle both line ending types. + self.tools._conn.exec_ps_command.return_value = ( + "\\\\?\\Volume{2750d574-b333-4e7b-a0a2-d739279d39e9}\\\r\n" + "\\\\?\\Volume{7723f315-c13c-450c-8be6-f58e06f4ad45}\\\r\n" + "\\\\?\\Volume{cb7399af-8f6a-4a7b-a55c-e885ec3ff5fd}\\\n" + ) + + exp_ret = [ + "\\\\?\\Volume{2750d574-b333-4e7b-a0a2-d739279d39e9}\\", + "\\\\?\\Volume{7723f315-c13c-450c-8be6-f58e06f4ad45}\\", + "\\\\?\\Volume{cb7399af-8f6a-4a7b-a55c-e885ec3ff5fd}\\", + ] + ret = self.tools._get_encrypted_volume_ids() + + self.assertEqual(exp_ret, ret) + self.tools._conn.exec_ps_command.assert_called_once_with( + 'gwmi -ns "Root\\CIMV2\\Security\\MicrosoftVolumeEncryption" ' + '-class Win32_EncryptableVolume | % {$_.DeviceID}') + + def test_unlock_encrypted_volume(self): + vol = "\\\\?\\Volume{2750d574-b333-4e7b-a0a2-d739279d39e9}\\" + password = "6010ba47-28e4-4105-8b0a-69eed0a54283" + + self.tools._unlock_encrypted_volume(vol, password) + + exp_cmd = 'manage-bde -unlock "%s" -RecoveryPassword "%s"' % ( + vol, password) + self.tools._conn.exec_ps_command.assert_called_once_with( + exp_cmd) + + def test_suspend_bitlocker(self): + vol = "\\\\?\\Volume{2750d574-b333-4e7b-a0a2-d739279d39e9}\\" + + self.tools._suspend_bitlocker(vol) + + exp_cmd = 'Suspend-BitLocker "%s"' % (vol) + self.tools._conn.exec_ps_command.assert_called_once_with( + exp_cmd) + + @mock.patch.object(windows.WindowsMountTools, "_get_encrypted_volume_ids") + def test_unlock_encrypted_volumes_no_password( + self, + mock_get_encrypted_volume_ids, + ): + self.tools._unlock_encrypted_volumes() + mock_get_encrypted_volume_ids.assert_not_called() + + @mock.patch.object(windows.WindowsMountTools, "_get_encrypted_volume_ids") + @mock.patch.object(windows.WindowsMountTools, "_unlock_encrypted_volume") + def test_unlock_encrypted_volumes_not_encrypted( + self, + mock_unlock_encrypted_volume, + mock_get_encrypted_volume_ids, + ): + fake_pass = "fake-recovery-password" + self.tools._osmorphing_info[constants.ENCRYPTED_DISKS_PASS] = fake_pass + + mock_get_encrypted_volume_ids.return_value = [] + + self.tools._unlock_encrypted_volumes() + mock_unlock_encrypted_volume.assert_not_called() + + @mock.patch.object(windows.WindowsMountTools, "_get_encrypted_volume_ids") + @mock.patch.object(windows.WindowsMountTools, "_unlock_encrypted_volume") + @mock.patch.object(windows.WindowsMountTools, "_suspend_bitlocker") + def test_unlock_encrypted_volumes_all_failed( + self, + mock_suspend_bitlocker, + mock_unlock_encrypted_volume, + mock_get_encrypted_volume_ids, + ): + fake_pass = "fake-recovery-password" + self.tools._osmorphing_info[constants.ENCRYPTED_DISKS_PASS] = fake_pass + + mock_get_encrypted_volume_ids.return_value = [ + mock.sentinel.volume0, + mock.sentinel.volume1, + ] + mock_unlock_encrypted_volume.side_effect = ValueError + + self.assertRaises( + exception.CoriolisException, + self.tools._unlock_encrypted_volumes, + ) + + @mock.patch.object(windows.WindowsMountTools, "_get_encrypted_volume_ids") + @mock.patch.object(windows.WindowsMountTools, "_unlock_encrypted_volume") + @mock.patch.object(windows.WindowsMountTools, "_suspend_bitlocker") + def test_unlock_encrypted_volumes_one_failed( + self, + mock_suspend_bitlocker, + mock_unlock_encrypted_volume, + mock_get_encrypted_volume_ids, + ): + fake_pass = "fake-recovery-password" + self.tools._osmorphing_info[constants.ENCRYPTED_DISKS_PASS] = fake_pass + + encrypted_volume_ids = [ + mock.sentinel.volume0, + mock.sentinel.volume1, + ] + mock_get_encrypted_volume_ids.return_value = encrypted_volume_ids + mock_unlock_encrypted_volume.side_effect = [ValueError, None] + + self.tools._unlock_encrypted_volumes() + + mock_unlock_encrypted_volume.assert_has_calls( + [mock.call(vol_id, fake_pass) for vol_id in encrypted_volume_ids]) + mock_suspend_bitlocker.assert_called_once_with( + mock.sentinel.volume1) + + @mock.patch.object(windows.WindowsMountTools, "_get_encrypted_volume_ids") + @mock.patch.object(windows.WindowsMountTools, "_unlock_encrypted_volume") + @mock.patch.object(windows.WindowsMountTools, "_suspend_bitlocker") + def test_unlock_encrypted_volumes_suspend_failed( + self, + mock_suspend_bitlocker, + mock_unlock_encrypted_volume, + mock_get_encrypted_volume_ids, + ): + fake_pass = "fake-recovery-password" + self.tools._osmorphing_info[constants.ENCRYPTED_DISKS_PASS] = fake_pass + + encrypted_volume_ids = [ + mock.sentinel.volume0, + mock.sentinel.volume1, + mock.sentinel.volume2 + ] + mock_get_encrypted_volume_ids.return_value = encrypted_volume_ids + mock_unlock_encrypted_volume.side_effect = [ValueError, None, None] + mock_suspend_bitlocker.side_effect = [IOError, None] + + # It should error out immediately when failing to suspend BitLocker. + self.assertRaises( + IOError, + self.tools._unlock_encrypted_volumes, + ) + mock_unlock_encrypted_volume.assert_has_calls( + [mock.call(mock.sentinel.volume0, fake_pass), + mock.call(mock.sentinel.volume1, fake_pass)] + ) diff --git a/coriolis/tests/osmorphing/test_manager.py b/coriolis/tests/osmorphing/test_manager.py index e85669b67..9b63b76f4 100644 --- a/coriolis/tests/osmorphing/test_manager.py +++ b/coriolis/tests/osmorphing/test_manager.py @@ -255,6 +255,7 @@ def test_morph_image( mock_get_os_mount_tools.assert_called_once_with( 'linux', mock.sentinel.connection_info, self.event_manager, [], 60, osmorphing_info=self.osmorphing_info) + mock_EventManager.assert_called_with(self.event_handler) self.os_mount_tools.dismount_os.assert_called_once() From 1dda26c47a995205c33f2f4d532325787dc16937 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Fri, 5 Jun 2026 13:26:29 +0000 Subject: [PATCH 2/5] Use first-boot script to resume BitLocker Unfortunately the "-RebootCount" parameter of "Suspend-BitLocker" isn't honored, perhaps due to the fact that the disks are attached to a different VM. For this reason, we'll inject a first-boot script to resume BitLocker explicitly. --- coriolis/osmorphing/osmount/windows.py | 43 ++++++++++++++++++- .../tests/osmorphing/osmount/test_windows.py | 25 +++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/coriolis/osmorphing/osmount/windows.py b/coriolis/osmorphing/osmount/windows.py index dd9c1ca2e..d2f03f283 100644 --- a/coriolis/osmorphing/osmount/windows.py +++ b/coriolis/osmorphing/osmount/windows.py @@ -16,6 +16,13 @@ class WindowsMountTools(base.BaseOSMountTools): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + # A list of BitLocker encrypted volumes that were unlocked + # by us. We'll use a first-boot script to resume BitLocker. + self._unlocked_volumes: list[str] = [] + def _connect(self): connection_info = self._connection_info @@ -241,8 +248,11 @@ def _suspend_bitlocker(self, volume_id: str): It doesn't decrypt the device, it just adds a publicly accessible BitLocker protector that automatically unlocks the volume. - When the replica instance boots, BitLocker will be automatically - enabled again and the TPM protector reconfigured. + When the replica instance boots, the TPM protector will be reconfigured + automatically. Unfortunately the '-RebootCount' parameter isn't + honored, perhaps due to the fact that the disks are attached to a + separate VM. For this reason, we'll use a first-boot script to resume + BitLocker explicitly. """ self._conn.exec_ps_command(f'Suspend-BitLocker "{volume_id}"') @@ -281,12 +291,41 @@ def _unlock_encrypted_volumes(self): # We'll intentionally propagate the failure if we managed to # unlock the volume but failed to suspend BitLocker. self._suspend_bitlocker(encrypted_volume_id) + self._unlocked_volumes.append(encrypted_volume_id) if not unlocked: raise exception.CoriolisException( "Could not unlock any volume using the specified " "BitLocker recovery password.") + def install_encryption_firstboot_setup( + self, + os_root_dir, + os_morphing_tools, + ): + if not self._unlocked_volumes: + LOG.info( + "No unlocked BitLocker volumes, skipping first-boot setup.") + return + + # We'll inject a first-boot script to resume BitLocker explicitly. + # Unfortunately the "-RebootCount" parameter of "Suspend-BitLocker" + # isn't honored, perhaps due to the fact that the disks are attached + # to a different VM. + script_content = "" + for encrypted_volume_id in self._unlocked_volumes: + LOG.info( + "Resuming BitLocker after first boot, volume: %s", + encrypted_volume_id) + script_content += f'Resume-BitLocker "{encrypted_volume_id}"\r\n' + + # Resume BitLocker after bringing the disks online, which has a script + # priority of 10. + os_morphing_tools.register_firstboot_script( + script_content, + user_provided=False, + script_filename="11-bitlocker-firstboot.ps1") + def mount_os(self): self._set_basic_disks_rw_mode() self._bring_disks_online() diff --git a/coriolis/tests/osmorphing/osmount/test_windows.py b/coriolis/tests/osmorphing/osmount/test_windows.py index b9eab554e..158109953 100644 --- a/coriolis/tests/osmorphing/osmount/test_windows.py +++ b/coriolis/tests/osmorphing/osmount/test_windows.py @@ -451,6 +451,9 @@ def test_unlock_encrypted_volumes_one_failed( mock_suspend_bitlocker.assert_called_once_with( mock.sentinel.volume1) + self.assertEqual( + self.tools._unlocked_volumes, [mock.sentinel.volume1]) + @mock.patch.object(windows.WindowsMountTools, "_get_encrypted_volume_ids") @mock.patch.object(windows.WindowsMountTools, "_unlock_encrypted_volume") @mock.patch.object(windows.WindowsMountTools, "_suspend_bitlocker") @@ -481,3 +484,25 @@ def test_unlock_encrypted_volumes_suspend_failed( [mock.call(mock.sentinel.volume0, fake_pass), mock.call(mock.sentinel.volume1, fake_pass)] ) + + def test_install_encryption_firstboot_setup_noop(self): + # No unlocked volumes, nothing to do. + mock_morphing_tools = mock.Mock() + self.tools.install_encryption_firstboot_setup( + mock.sentinel.os_root_dir, + mock_morphing_tools) + mock_morphing_tools.register_firstboot_script.assert_not_called() + + def test_install_encryption_firstboot_setup(self): + self.tools._unlocked_volumes = ["vol1", "vol2"] + mock_morphing_tools = mock.Mock() + self.tools.install_encryption_firstboot_setup( + mock.sentinel.os_root_dir, + mock_morphing_tools) + + expected_script = ( + 'Resume-BitLocker "vol1"\r\nResume-BitLocker "vol2"\r\n') + mock_morphing_tools.register_firstboot_script.assert_called_once_with( + expected_script, + user_provided=False, + script_filename="11-bitlocker-firstboot.ps1") From 8f3ae63f8f4d32284f5e63d52743645bdb795098 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Mon, 8 Jun 2026 10:35:45 +0000 Subject: [PATCH 3/5] Sanitize commands before logging them Coriolis currently logs sensitive information, the executed PowerShell commands among other things: https://github.com/cloudbase/coriolis/pull/450#discussion_r3371641045 We'll reuse the sanitization helpers from oslo.utils. --- .../tests/osmorphing/osmount/test_windows.py | 2 ++ coriolis/tests/osmorphing/test_manager.py | 14 ++++---- coriolis/tests/test_wsman.py | 16 +++++++--- coriolis/utils.py | 6 ++-- coriolis/wsman.py | 32 ++++++++++++++----- 5 files changed, 49 insertions(+), 21 deletions(-) diff --git a/coriolis/tests/osmorphing/osmount/test_windows.py b/coriolis/tests/osmorphing/osmount/test_windows.py index 158109953..7dbe3b876 100644 --- a/coriolis/tests/osmorphing/osmount/test_windows.py +++ b/coriolis/tests/osmorphing/osmount/test_windows.py @@ -4,6 +4,8 @@ import logging from unittest import mock +from oslo_utils import strutils + from coriolis import constants from coriolis import exception from coriolis.osmorphing.osmount import windows diff --git a/coriolis/tests/osmorphing/test_manager.py b/coriolis/tests/osmorphing/test_manager.py index 9b63b76f4..8231ee5f9 100644 --- a/coriolis/tests/osmorphing/test_manager.py +++ b/coriolis/tests/osmorphing/test_manager.py @@ -76,16 +76,16 @@ def test_run_os_detect(self, mock_detect_os): result = manager.run_os_detect( self.provider, self.destination_provider, self.worker_connection, mock.sentinel.os_type, mock.sentinel.os_root_dir, - mock.sentinel.osmorphing_info, tools_environment={}) + self.osmorphing_info, tools_environment={}) self.assertEqual(result, mock_detect_os.return_value) self.provider.get_custom_os_detect_tools.\ assert_called_once_with(mock.sentinel.os_type, - mock.sentinel.osmorphing_info) + self.osmorphing_info) self.destination_provider.get_custom_os_detect_tools.\ assert_called_once_with(mock.sentinel.os_type, - mock.sentinel.osmorphing_info) + self.osmorphing_info) mock_detect_os.assert_called_once_with( self.worker_connection, mock.sentinel.os_type, mock.sentinel.os_root_dir, @@ -112,7 +112,7 @@ def check_os_supported(cls, detected_os_info): result = manager.get_osmorphing_tools_class_for_provider( self.provider, mock.sentinel.detected_os_info, - mock.sentinel.os_type, mock.sentinel.osmorphing_info) + mock.sentinel.os_type, self.osmorphing_info) self.assertEqual(result, MockToolsClass) @@ -126,7 +126,7 @@ class MockInvalidToolsClass: self.assertRaises(exception.InvalidOSMorphingTools, manager.get_osmorphing_tools_class_for_provider, self.provider, mock.sentinel.detected_os_info, - mock.sentinel.os_type, mock.sentinel.osmorphing_info) + mock.sentinel.os_type, self.osmorphing_info) def test_get_osmorphing_tools_class_for_provider_invalid_os_params(self): class MockToolsClass(base_osmorphing.BaseOSMorphingTools): @@ -144,7 +144,7 @@ def check_detected_os_info_parameters(cls, detected_os_info): 'coriolis.osmorphing.manager', level=logging.WARN): result = manager.get_osmorphing_tools_class_for_provider( self.provider, mock.sentinel.detected_os_info, - mock.sentinel.os_type, mock.sentinel.osmorphing_info) + mock.sentinel.os_type, self.osmorphing_info) self.assertIsNone(result) @@ -164,7 +164,7 @@ def check_os_supported(cls, detected_os_info): 'coriolis.osmorphing.manager', level=logging.DEBUG): result = manager.get_osmorphing_tools_class_for_provider( self.provider, self.detected_os_info, - mock.sentinel.os_type, mock.sentinel.osmorphing_info) + mock.sentinel.os_type, self.osmorphing_info) self.assertIsNone(result) diff --git a/coriolis/tests/test_wsman.py b/coriolis/tests/test_wsman.py index 1683b3f5c..d957e50ba 100644 --- a/coriolis/tests/test_wsman.py +++ b/coriolis/tests/test_wsman.py @@ -1,6 +1,7 @@ # Copyright 2023 Cloudbase Solutions Srl # All Rights Reserved. +import logging from unittest import mock import requests @@ -20,7 +21,8 @@ def setUp(self): self.conn._protocol = mock.Mock() self.conn._conn_timeout = 10 self.cmd = "test_cmd" - self.args = ["arg1", "arg2"] + self.args = ["-RecoveryPassword", "'ShouldNotBeLogged'"] + self.sanitized_cmd = "test_cmd -RecoveryPassword '***'" self.url = "http://example.com/file" self.remote_path = "/remote/path" @@ -134,8 +136,13 @@ def test__exec_command_invalid_credentials(self, mock_sleep): def test_exec_command(self): self.conn._protocol.get_command_output.return_value = ( "std_out", "std_err", 0) - std_out = self.conn.exec_command(self.cmd, self.args) - self.assertEqual(std_out, "std_out") + exp_sanitized_log = ( + "DEBUG:coriolis.wsman:Executing WSMAN command: %s" % + self.sanitized_cmd) + with self.assertLogs("coriolis.wsman", level=logging.DEBUG) as log_cm: + std_out = self.conn.exec_command(self.cmd, self.args) + self.assertEqual(std_out, "std_out") + self.assertIn(exp_sanitized_log, log_cm.output) def test_exec_command_exception(self): self.conn._protocol.get_command_output.return_value = ( @@ -155,7 +162,8 @@ def test_exec_ps_command(self): '-NonInteractive', '-ExecutionPolicy', 'RemoteSigned', ], - timeout=None) + timeout=None, + sanitizable=False) self.assertEqual(result, "std_out") def test_test_path(self): diff --git a/coriolis/utils.py b/coriolis/utils.py index 823544715..e9f3b0b7a 100644 --- a/coriolis/utils.py +++ b/coriolis/utils.py @@ -27,6 +27,7 @@ from oslo_config import cfg from oslo_log import log as logging from oslo_serialization import jsonutils +from oslo_utils import strutils import netifaces import paramiko @@ -316,6 +317,7 @@ def list_ssh_dir(ssh, remote_path): def _exec_ssh_cmd(ssh, cmd, environment=None, get_pty=False, timeout=None): + sanitized_cmd = strutils.mask_password(cmd) remote_str = "" if timeout is not None: timeout = float(timeout) @@ -327,7 +329,7 @@ def _exec_ssh_cmd(ssh, cmd, environment=None, get_pty=False, timeout=None): get_exception_details()) LOG.debug( "Executing the following SSH command on '%s' with " - "environment %s: '%s'", remote_str, environment, cmd) + "environment %s: '%s'", remote_str, environment, sanitized_cmd) _, stdout, stderr = ssh.exec_command( cmd, environment=environment, get_pty=get_pty, timeout=timeout) @@ -343,7 +345,7 @@ def _exec_ssh_cmd(ssh, cmd, environment=None, get_pty=False, timeout=None): msg = ( "Command \"%s\" failed on host '%s' with exit code: %s\n" "stdout: %s\nstd_err: %s" % - (cmd, remote_str, exit_code, stdout_str, stderr_str)) + (sanitized_cmd, remote_str, exit_code, stdout_str, stderr_str)) if (exit_code == 127 or "command not found" in stdout_str or "command not found" in stderr_str): diff --git a/coriolis/wsman.py b/coriolis/wsman.py index 02bba720d..4638dba9f 100644 --- a/coriolis/wsman.py +++ b/coriolis/wsman.py @@ -5,6 +5,7 @@ import requests from oslo_log import log as logging +from oslo_utils import strutils from winrm import exceptions as winrm_exceptions from winrm import protocol @@ -97,7 +98,13 @@ def set_timeout(self, timeout): @utils.retry_on_error( terminal_exceptions=[winrm_exceptions.InvalidCredentialsError, exception.OSMorphingWinRMOperationTimeout]) - def _exec_command(self, cmd, args=[], timeout=None): + def _exec_command(self, cmd, args=[], timeout=None, sanitizable=True): + if sanitizable: + sanitized_cmd = strutils.mask_password( + "%s %s" % (cmd, " ".join(args))) + else: + sanitized_cmd = "***" + timeout = int(timeout or self._conn_timeout) self.set_timeout(timeout) shell_id = None @@ -111,7 +118,7 @@ def _exec_command(self, cmd, args=[], timeout=None): shell_id, command_id) except requests.exceptions.ReadTimeout: raise exception.OSMorphingWinRMOperationTimeout( - cmd=("%s %s" % (cmd, " ".join(args))), timeout=timeout) + cmd=sanitized_cmd, timeout=timeout) finally: self._protocol.cleanup_command(shell_id, command_id) @@ -132,21 +139,29 @@ def _exec_command(self, cmd, args=[], timeout=None): if shell_id: self._protocol.close_shell(shell_id) - def exec_command(self, cmd, args=[], timeout=None): - LOG.debug("Executing WSMAN command: %s", str([cmd] + args)) + def exec_command(self, cmd, args=[], timeout=None, sanitizable=True): + # Our sanitization helpers do not work for base64 encoded commands, + # in which case we'll avoid logging it so that we won't leak + # sensitive information. + if sanitizable: + sanitized_cmd = strutils.mask_password( + "%s %s" % (cmd, " ".join(args))) + else: + sanitized_cmd = "***" + LOG.debug("Executing WSMAN command: %s", sanitized_cmd) std_out, std_err, exit_code = self._exec_command( - cmd, args, timeout=timeout) + cmd, args, timeout=timeout, sanitizable=sanitizable) if exit_code: raise exception.CoriolisException( "Command \"%s\" failed with exit code: %s\n" "stdout: %s\nstd_err: %s" % - (str([cmd] + args), exit_code, std_out, std_err)) + (sanitized_cmd, exit_code, std_out, std_err)) return std_out def exec_ps_command(self, cmd, ignore_stdout=False, timeout=None): - LOG.debug("Executing PS command: %s", cmd) + LOG.debug("Executing PS command: %s", strutils.mask_password(cmd)) base64_cmd = base64.b64encode(cmd.encode('utf-16le')).decode() return self.exec_command( "powershell.exe", @@ -157,7 +172,8 @@ def exec_ps_command(self, cmd, ignore_stdout=False, timeout=None): "-ExecutionPolicy", "RemoteSigned", ], - timeout=timeout)[:-2] + timeout=timeout, + sanitizable=False)[:-2] def test_path(self, remote_path): ret_val = self.exec_ps_command("Test-Path -Path \"%s\"" % remote_path) From 876028fa6edac0bb14b693935c50e8cbbe1467c8 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Mon, 8 Jun 2026 10:41:51 +0000 Subject: [PATCH 4/5] Sanitize task info and os morphing info dicts We'll use "mask_dict_password" from oslo.utils to sanitize task info and os morphing info dicts. This covers a wide variety of keys that are expected to contain sensitive data, including the ones used for BitLocker and LUKS keys. --- coriolis/minion_manager/rpc/server.py | 5 ++++- coriolis/osmorphing/manager.py | 4 +++- coriolis/tests/osmorphing/osmount/test_windows.py | 11 +++++++++++ coriolis/tests/worker/rpc/test_server.py | 6 ++++-- coriolis/utils.py | 1 + coriolis/worker/rpc/server.py | 4 +++- 6 files changed, 26 insertions(+), 5 deletions(-) diff --git a/coriolis/minion_manager/rpc/server.py b/coriolis/minion_manager/rpc/server.py index dd3899c62..af7caf387 100644 --- a/coriolis/minion_manager/rpc/server.py +++ b/coriolis/minion_manager/rpc/server.py @@ -7,6 +7,7 @@ from oslo_config import cfg from oslo_log import log as logging +from oslo_utils import strutils from oslo_utils import timeutils from taskflow import deciders as taskflow_deciders from taskflow.patterns import graph_flow @@ -508,7 +509,9 @@ def _check_pool_minion_count( pool_id, instances, action['id'])) LOG.debug( "Successfully validated minion pool selections for action '%s' " - "with properties: %s", action['id'], action) + "with properties: %s", + action['id'], + strutils.mask_dict_password(action)) def allocate_minion_machines_for_transfer( self, ctxt, transfer): diff --git a/coriolis/osmorphing/manager.py b/coriolis/osmorphing/manager.py index a374a0ba5..679e88650 100644 --- a/coriolis/osmorphing/manager.py +++ b/coriolis/osmorphing/manager.py @@ -5,6 +5,7 @@ from oslo_config import cfg from oslo_log import log as logging +from oslo_utils import strutils from coriolis import constants from coriolis import events @@ -84,7 +85,8 @@ def get_osmorphing_tools_class_for_provider( LOG.debug( "OSMorphing tools classes returned by provider '%s' for os_type '%s' " "and 'osmorphing_info' %s: %s", - type(provider), os_type, osmorphing_info, available_tools_cls) + type(provider), os_type, + strutils.mask_dict_password(osmorphing_info), available_tools_cls) osmorphing_base_class = base_osmorphing.BaseOSMorphingTools for toolscls in available_tools_cls: diff --git a/coriolis/tests/osmorphing/osmount/test_windows.py b/coriolis/tests/osmorphing/osmount/test_windows.py index 7dbe3b876..8054b91b0 100644 --- a/coriolis/tests/osmorphing/osmount/test_windows.py +++ b/coriolis/tests/osmorphing/osmount/test_windows.py @@ -372,6 +372,17 @@ def test_unlock_encrypted_volume(self): self.tools._conn.exec_ps_command.assert_called_once_with( exp_cmd) + def test_sanitize_recovery_password(self): + vol = "\\\\?\\Volume{2750d574-b333-4e7b-a0a2-d739279d39e9}\\" + password = "6010ba47-28e4-4105-8b0a-69eed0a54283" + + cmd = 'manage-bde -unlock "%s" -RecoveryPassword "%s"' % ( + vol, password) + exp_cmd = 'manage-bde -unlock "%s" -RecoveryPassword "%s"' % ( + vol, '***') + + self.assertEqual(exp_cmd, strutils.mask_password(exp_cmd)) + def test_suspend_bitlocker(self): vol = "\\\\?\\Volume{2750d574-b333-4e7b-a0a2-d739279d39e9}\\" diff --git a/coriolis/tests/worker/rpc/test_server.py b/coriolis/tests/worker/rpc/test_server.py index 5c13ceeb0..81747b6d2 100644 --- a/coriolis/tests/worker/rpc/test_server.py +++ b/coriolis/tests/worker/rpc/test_server.py @@ -1265,9 +1265,11 @@ def test__task_process(self, mock_is_serializable, mock_task_runner = mock_get_task_runner_class.return_value.return_value mock_task_result = mock_task_runner.run.return_value + mock_destination = {'connection_info': "fake-conn-info"} + server._task_process(mock.sentinel.ctxt, mock.sentinel.task_id, mock.sentinel.task_type, mock.sentinel.origin, - mock.sentinel.destination, mock.sentinel.instance, + mock_destination, mock.sentinel.instance, task_info, mp_q, mp_log_q) mock_setup_task_process.assert_called_once_with(mp_log_q) mock_get_task_runner_class.assert_called_once_with( @@ -1277,7 +1279,7 @@ def test__task_process(self, mock_is_serializable, mock.sentinel.task_id) mock_task_runner.run.assert_called_once_with( mock.sentinel.ctxt, mock.sentinel.instance, mock.sentinel.origin, - mock.sentinel.destination, task_info, + mock_destination, task_info, mock_get_event_handler.return_value) mock_is_serializable.assert_called_once_with(mock_task_result) mp_q.put.assert_called_once_with(mock_task_result) diff --git a/coriolis/utils.py b/coriolis/utils.py index e9f3b0b7a..660175ef5 100644 --- a/coriolis/utils.py +++ b/coriolis/utils.py @@ -765,6 +765,7 @@ def sanitize_task_info(task_info): [""]) new['volumes_info'].append(vol_cpy) + new = strutils.mask_dict_password(new) return new diff --git a/coriolis/worker/rpc/server.py b/coriolis/worker/rpc/server.py index cae63b6f0..befcdc8eb 100644 --- a/coriolis/worker/rpc/server.py +++ b/coriolis/worker/rpc/server.py @@ -12,6 +12,7 @@ from oslo_config import cfg from oslo_log import log as logging +from oslo_utils import strutils import psutil from six.moves import queue @@ -688,7 +689,8 @@ def _task_process(ctxt, task_id, task_type, origin, destination, instance, "origin: %(origin)s, destination: %(destination)s, " "instance: %(instance)s, task_info: %(task_info)s", {"task_id": task_id, "task_type": task_type, - "origin": origin, "destination": destination, + "origin": origin, + "destination": strutils.mask_dict_password(destination), "instance": instance, "task_info": utils.sanitize_task_info( task_info)}) From 63a637a3b9f974bc6891020e454cfcb31ac21582 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Mon, 8 Jun 2026 12:13:12 +0000 Subject: [PATCH 5/5] Ensure that Bitlocker is resumed in case of failures We need to resume BitLocker if the os-morphing process fails, otherwise the disks will remain publicly open. "install_encryption_firstboot_setup" is the last method called during os-morphing, we can suspend Bitlocker there and resume it in case of failures. While at it, we'll move "_unlock_encrypted_volumes" next to "_unlock_encrypted_volume". --- coriolis/osmorphing/osmount/windows.py | 79 +++++++++------ .../tests/osmorphing/osmount/test_windows.py | 98 +++++++++++-------- 2 files changed, 106 insertions(+), 71 deletions(-) diff --git a/coriolis/osmorphing/osmount/windows.py b/coriolis/osmorphing/osmount/windows.py index d2f03f283..57e68a01c 100644 --- a/coriolis/osmorphing/osmount/windows.py +++ b/coriolis/osmorphing/osmount/windows.py @@ -242,20 +242,6 @@ def _unlock_encrypted_volume(self, volume_id: str, recovery_password: str): f'manage-bde -unlock "{volume_id}" ' f'-RecoveryPassword "{recovery_password}"') - def _suspend_bitlocker(self, volume_id: str): - """Suspend BitLocker until the next reboot for a given volume. - - It doesn't decrypt the device, it just adds a publicly accessible - BitLocker protector that automatically unlocks the volume. - - When the replica instance boots, the TPM protector will be reconfigured - automatically. Unfortunately the '-RebootCount' parameter isn't - honored, perhaps due to the fact that the disks are attached to a - separate VM. For this reason, we'll use a first-boot script to resume - BitLocker explicitly. - """ - self._conn.exec_ps_command(f'Suspend-BitLocker "{volume_id}"') - def _unlock_encrypted_volumes(self): recovery_password = self._osmorphing_info.get( constants.ENCRYPTED_DISKS_PASS) @@ -285,12 +271,6 @@ def _unlock_encrypted_volumes(self): "recovery password.", encrypted_volume_id) continue - - # Suspend BitLocker until the replica boots. - # - # We'll intentionally propagate the failure if we managed to - # unlock the volume but failed to suspend BitLocker. - self._suspend_bitlocker(encrypted_volume_id) self._unlocked_volumes.append(encrypted_volume_id) if not unlocked: @@ -298,6 +278,23 @@ def _unlock_encrypted_volumes(self): "Could not unlock any volume using the specified " "BitLocker recovery password.") + def _suspend_bitlocker(self, volume_id: str): + """Suspend BitLocker until the next reboot for a given volume. + + It doesn't decrypt the device, it just adds a publicly accessible + BitLocker protector that automatically unlocks the volume. + + When the replica instance boots, the TPM protector will be reconfigured + automatically. Unfortunately the '-RebootCount' parameter isn't + honored, perhaps due to the fact that the disks are attached to a + separate VM. For this reason, we'll use a first-boot script to resume + BitLocker explicitly. + """ + self._conn.exec_ps_command(f'Suspend-BitLocker "{volume_id}"') + + def _resume_bitlocker(self, volume_id: str): + self._conn.exec_ps_command(f'Resume-BitLocker "{volume_id}"') + def install_encryption_firstboot_setup( self, os_root_dir, @@ -313,18 +310,36 @@ def install_encryption_firstboot_setup( # isn't honored, perhaps due to the fact that the disks are attached # to a different VM. script_content = "" - for encrypted_volume_id in self._unlocked_volumes: - LOG.info( - "Resuming BitLocker after first boot, volume: %s", - encrypted_volume_id) - script_content += f'Resume-BitLocker "{encrypted_volume_id}"\r\n' - - # Resume BitLocker after bringing the disks online, which has a script - # priority of 10. - os_morphing_tools.register_firstboot_script( - script_content, - user_provided=False, - script_filename="11-bitlocker-firstboot.ps1") + + try: + for encrypted_volume_id in self._unlocked_volumes: + LOG.info( + "Suspending BitLocker for volume %s, scheduling it to be " + "resumed after first-boot.", + encrypted_volume_id) + # Suspend BitLocker until the replica boots. + self._suspend_bitlocker(encrypted_volume_id) + # Add a resume command to the first-boot script. + script_content += ( + f'Resume-BitLocker "{encrypted_volume_id}"\r\n') + + # Resume BitLocker after bringing the disks online, + # which has a script priority of 10. + os_morphing_tools.register_firstboot_script( + script_content, + user_provided=False, + script_filename="11-bitlocker-firstboot.ps1") + except Exception: + LOG.exception("First-boot preparation failed, attempting to " + "resume BitLocker.") + for encrypted_volume_id in self._unlocked_volumes: + try: + self._resume_bitlocker(encrypted_volume_id) + except Exception: + LOG.exception( + "Unable to resume BitLocker for volume: %s" % + encrypted_volume_id) + raise def mount_os(self): self._set_basic_disks_rw_mode() diff --git a/coriolis/tests/osmorphing/osmount/test_windows.py b/coriolis/tests/osmorphing/osmount/test_windows.py index 8054b91b0..dfe909cd1 100644 --- a/coriolis/tests/osmorphing/osmount/test_windows.py +++ b/coriolis/tests/osmorphing/osmount/test_windows.py @@ -381,7 +381,7 @@ def test_sanitize_recovery_password(self): exp_cmd = 'manage-bde -unlock "%s" -RecoveryPassword "%s"' % ( vol, '***') - self.assertEqual(exp_cmd, strutils.mask_password(exp_cmd)) + self.assertEqual(exp_cmd, strutils.mask_password(cmd)) def test_suspend_bitlocker(self): vol = "\\\\?\\Volume{2750d574-b333-4e7b-a0a2-d739279d39e9}\\" @@ -392,6 +392,15 @@ def test_suspend_bitlocker(self): self.tools._conn.exec_ps_command.assert_called_once_with( exp_cmd) + def test_resume_bitlocker(self): + vol = "\\\\?\\Volume{2750d574-b333-4e7b-a0a2-d739279d39e9}\\" + + self.tools._resume_bitlocker(vol) + + exp_cmd = 'Resume-BitLocker "%s"' % (vol) + self.tools._conn.exec_ps_command.assert_called_once_with( + exp_cmd) + @mock.patch.object(windows.WindowsMountTools, "_get_encrypted_volume_ids") def test_unlock_encrypted_volumes_no_password( self, @@ -417,10 +426,8 @@ def test_unlock_encrypted_volumes_not_encrypted( @mock.patch.object(windows.WindowsMountTools, "_get_encrypted_volume_ids") @mock.patch.object(windows.WindowsMountTools, "_unlock_encrypted_volume") - @mock.patch.object(windows.WindowsMountTools, "_suspend_bitlocker") def test_unlock_encrypted_volumes_all_failed( self, - mock_suspend_bitlocker, mock_unlock_encrypted_volume, mock_get_encrypted_volume_ids, ): @@ -440,10 +447,8 @@ def test_unlock_encrypted_volumes_all_failed( @mock.patch.object(windows.WindowsMountTools, "_get_encrypted_volume_ids") @mock.patch.object(windows.WindowsMountTools, "_unlock_encrypted_volume") - @mock.patch.object(windows.WindowsMountTools, "_suspend_bitlocker") def test_unlock_encrypted_volumes_one_failed( self, - mock_suspend_bitlocker, mock_unlock_encrypted_volume, mock_get_encrypted_volume_ids, ): @@ -461,43 +466,10 @@ def test_unlock_encrypted_volumes_one_failed( mock_unlock_encrypted_volume.assert_has_calls( [mock.call(vol_id, fake_pass) for vol_id in encrypted_volume_ids]) - mock_suspend_bitlocker.assert_called_once_with( - mock.sentinel.volume1) self.assertEqual( self.tools._unlocked_volumes, [mock.sentinel.volume1]) - @mock.patch.object(windows.WindowsMountTools, "_get_encrypted_volume_ids") - @mock.patch.object(windows.WindowsMountTools, "_unlock_encrypted_volume") - @mock.patch.object(windows.WindowsMountTools, "_suspend_bitlocker") - def test_unlock_encrypted_volumes_suspend_failed( - self, - mock_suspend_bitlocker, - mock_unlock_encrypted_volume, - mock_get_encrypted_volume_ids, - ): - fake_pass = "fake-recovery-password" - self.tools._osmorphing_info[constants.ENCRYPTED_DISKS_PASS] = fake_pass - - encrypted_volume_ids = [ - mock.sentinel.volume0, - mock.sentinel.volume1, - mock.sentinel.volume2 - ] - mock_get_encrypted_volume_ids.return_value = encrypted_volume_ids - mock_unlock_encrypted_volume.side_effect = [ValueError, None, None] - mock_suspend_bitlocker.side_effect = [IOError, None] - - # It should error out immediately when failing to suspend BitLocker. - self.assertRaises( - IOError, - self.tools._unlock_encrypted_volumes, - ) - mock_unlock_encrypted_volume.assert_has_calls( - [mock.call(mock.sentinel.volume0, fake_pass), - mock.call(mock.sentinel.volume1, fake_pass)] - ) - def test_install_encryption_firstboot_setup_noop(self): # No unlocked volumes, nothing to do. mock_morphing_tools = mock.Mock() @@ -506,7 +478,8 @@ def test_install_encryption_firstboot_setup_noop(self): mock_morphing_tools) mock_morphing_tools.register_firstboot_script.assert_not_called() - def test_install_encryption_firstboot_setup(self): + @mock.patch.object(windows.WindowsMountTools, "_suspend_bitlocker") + def test_install_encryption_firstboot_setup(self, mock_suspend_bitlocker): self.tools._unlocked_volumes = ["vol1", "vol2"] mock_morphing_tools = mock.Mock() self.tools.install_encryption_firstboot_setup( @@ -519,3 +492,50 @@ def test_install_encryption_firstboot_setup(self): expected_script, user_provided=False, script_filename="11-bitlocker-firstboot.ps1") + mock_suspend_bitlocker.assert_has_calls( + [mock.call(volume) for volume in self.tools._unlocked_volumes]) + + @mock.patch.object(windows.WindowsMountTools, "_suspend_bitlocker") + @mock.patch.object(windows.WindowsMountTools, "_resume_bitlocker") + def test_install_encryption_firstboot_setup_register_failure( + self, + mock_resume_bitlocker, + mock_suspend_bitlocker + ): + self.tools._unlocked_volumes = ["vol1", "vol2"] + mock_morphing_tools = mock.Mock() + mock_morphing_tools.register_firstboot_script.side_effect = IOError + self.assertRaises( + IOError, + self.tools.install_encryption_firstboot_setup, + mock.sentinel.os_root_dir, + mock_morphing_tools) + + mock_suspend_bitlocker.assert_has_calls( + [mock.call(volume) for volume in self.tools._unlocked_volumes]) + mock_resume_bitlocker.assert_has_calls( + [mock.call(volume) for volume in self.tools._unlocked_volumes]) + + @mock.patch.object(windows.WindowsMountTools, "_suspend_bitlocker") + @mock.patch.object(windows.WindowsMountTools, "_resume_bitlocker") + def test_install_encryption_firstboot_setup_suspend_failure( + self, + mock_resume_bitlocker, + mock_suspend_bitlocker + ): + self.tools._unlocked_volumes = ["vol1", "vol2"] + mock_morphing_tools = mock.Mock() + mock_suspend_bitlocker.side_effect = [None, IOError] + # Let resume-bitlocker fail to ensure that we're still trying to + # cover all volumes. + mock_resume_bitlocker.side_effect = ValueError + self.assertRaises( + IOError, + self.tools.install_encryption_firstboot_setup, + mock.sentinel.os_root_dir, + mock_morphing_tools) + + mock_suspend_bitlocker.assert_has_calls( + [mock.call(volume) for volume in self.tools._unlocked_volumes]) + mock_resume_bitlocker.assert_has_calls( + [mock.call(volume) for volume in self.tools._unlocked_volumes])