encryptors: Remove workaround for bug #1633518
This has been in place since Newton and was targeted for removal in Rocky. That has long since passed but we can ease things by removing it now. Despite what change I7096463c5eba951dd6322ee6965435e877ca0371 said about not removing this workaround from 'CryptsetupEncryptor', we do so anyway since we're going to remove this encryptor shortly. Change-Id: Id8d96c1eda2beab1aebd8ecc054d8511ccefca3e Signed-off-by: Stephen Finucane <sfinucan@redhat.com> Related-bug: #1633518
This commit is contained in:
parent
1b2e229542
commit
4c654d9454
|
@ -13,8 +13,6 @@
|
|||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
|
||||
import array
|
||||
import binascii
|
||||
import os
|
||||
|
||||
|
@ -128,18 +126,6 @@ class CryptsetupEncryptor(base.VolumeEncryptor):
|
|||
check_exit_code=True, run_as_root=True,
|
||||
root_helper=self._root_helper)
|
||||
|
||||
def _get_mangled_passphrase(self, key):
|
||||
"""Convert the raw key into a list of unsigned int's and then a string
|
||||
|
||||
"""
|
||||
# NOTE(lyarwood): This replicates the methods used prior to Newton to
|
||||
# first encode the passphrase as a list of unsigned int's before
|
||||
# decoding back into a string. This method strips any leading 0's
|
||||
# of the resulting hex digit pairs, resulting in a different
|
||||
# passphrase being returned.
|
||||
encoded_key = array.array('B', key).tolist()
|
||||
return ''.join(hex(x).replace('0x', '') for x in encoded_key)
|
||||
|
||||
def attach_volume(self, context, **kwargs):
|
||||
"""Shadow the device and pass an unencrypted version to the instance.
|
||||
|
||||
|
@ -160,16 +146,7 @@ class CryptsetupEncryptor(base.VolumeEncryptor):
|
|||
key = self._get_key(context).get_encoded()
|
||||
passphrase = self._get_passphrase(key)
|
||||
|
||||
try:
|
||||
self._open_volume(passphrase, **kwargs)
|
||||
except processutils.ProcessExecutionError as e:
|
||||
if e.exit_code == 2:
|
||||
# NOTE(lyarwood): Workaround bug#1633518 by attempting to use
|
||||
# a mangled passphrase to open the device..
|
||||
LOG.info("Unable to open %s with the current passphrase, "
|
||||
"attempting to use a mangled passphrase to open "
|
||||
"the volume.", self.dev_path)
|
||||
self._open_volume(self._get_mangled_passphrase(key), **kwargs)
|
||||
self._open_volume(passphrase, **kwargs)
|
||||
|
||||
# modify the original symbolic link to refer to the decrypted device
|
||||
self._execute('ln', '--symbolic', '--force',
|
||||
|
|
|
@ -113,45 +113,6 @@ class LuksEncryptor(cryptsetup.CryptsetupEncryptor):
|
|||
run_as_root=True, check_exit_code=True,
|
||||
root_helper=self._root_helper)
|
||||
|
||||
def _unmangle_volume(self, key, passphrase, **kwargs):
|
||||
"""Workaround for bug#1633518
|
||||
|
||||
First identify if a mangled passphrase is used and if found then
|
||||
replace with the correct unmangled version of the passphrase.
|
||||
"""
|
||||
mangled_passphrase = self._get_mangled_passphrase(key)
|
||||
self._open_volume(mangled_passphrase, **kwargs)
|
||||
self._close_volume(**kwargs)
|
||||
LOG.debug("%s correctly opened with a mangled passphrase, replacing "
|
||||
"this with the original passphrase", self.dev_path)
|
||||
|
||||
# NOTE(lyarwood): Now that we are sure that the mangled passphrase is
|
||||
# used attempt to add the correct passphrase before removing the
|
||||
# mangled version from the volume.
|
||||
|
||||
# luksAddKey currently prompts for the following input :
|
||||
# Enter any existing passphrase:
|
||||
# Enter new passphrase for key slot:
|
||||
# Verify passphrase:
|
||||
self._execute('cryptsetup', 'luksAddKey', self.dev_path,
|
||||
'--force-password',
|
||||
process_input=''.join([mangled_passphrase, '\n',
|
||||
passphrase, '\n', passphrase]),
|
||||
run_as_root=True, check_exit_code=True,
|
||||
root_helper=self._root_helper)
|
||||
|
||||
# Verify that we can open the volume with the current passphrase
|
||||
# before removing the mangled passphrase.
|
||||
self._open_volume(passphrase, **kwargs)
|
||||
self._close_volume(**kwargs)
|
||||
|
||||
# luksRemoveKey only prompts for the key to remove.
|
||||
self._execute('cryptsetup', 'luksRemoveKey', self.dev_path,
|
||||
process_input=mangled_passphrase,
|
||||
run_as_root=True, check_exit_code=True,
|
||||
root_helper=self._root_helper)
|
||||
LOG.debug("%s mangled passphrase successfully replaced", self.dev_path)
|
||||
|
||||
def attach_volume(self, context, **kwargs):
|
||||
"""Shadow the device and pass an unencrypted version to the instance.
|
||||
|
||||
|
@ -176,16 +137,6 @@ class LuksEncryptor(cryptsetup.CryptsetupEncryptor):
|
|||
self.dev_path)
|
||||
self._format_volume(passphrase, **kwargs)
|
||||
self._open_volume(passphrase, **kwargs)
|
||||
elif e.exit_code == 2:
|
||||
# NOTE(lyarwood): Workaround bug#1633518 by replacing any
|
||||
# mangled passphrases that are found on the volume.
|
||||
# TODO(lyarwood): Remove workaround during R.
|
||||
LOG.warning("%s is not usable with the current "
|
||||
"passphrase, attempting to use a mangled "
|
||||
"passphrase to open the volume.",
|
||||
self.dev_path)
|
||||
self._unmangle_volume(key, passphrase, **kwargs)
|
||||
self._open_volume(passphrase, **kwargs)
|
||||
else:
|
||||
raise
|
||||
|
||||
|
|
|
@ -20,7 +20,6 @@ from unittest import mock
|
|||
|
||||
from castellan.common.objects import symmetric_key as key
|
||||
from castellan.tests.unit.key_manager import fake
|
||||
from oslo_concurrency import processutils as putils
|
||||
|
||||
from os_brick.encryptors import cryptsetup
|
||||
from os_brick import exception
|
||||
|
@ -154,34 +153,3 @@ class CryptsetupEncryptorTestCase(test_base.VolumeEncryptorTestCase):
|
|||
mock.call('/dev/mapper/%s' % wwn)])
|
||||
mock_execute.assert_called_once_with(
|
||||
'cryptsetup', 'status', wwn, run_as_root=True)
|
||||
|
||||
@mock.patch('os_brick.executor.Executor._execute')
|
||||
def test_attach_volume_unmangle_passphrase(self, mock_execute):
|
||||
fake_key = '0725230b'
|
||||
fake_key_mangled = '72523b'
|
||||
self.encryptor._get_key = mock.MagicMock()
|
||||
self.encryptor._get_key.return_value = fake__get_key(None, fake_key)
|
||||
|
||||
mock_execute.side_effect = [
|
||||
putils.ProcessExecutionError(exit_code=2), # luksOpen
|
||||
mock.DEFAULT,
|
||||
mock.DEFAULT,
|
||||
]
|
||||
|
||||
self.encryptor.attach_volume(None)
|
||||
|
||||
mock_execute.assert_has_calls([
|
||||
mock.call('cryptsetup', 'create', '--key-file=-', self.dev_name,
|
||||
self.dev_path, process_input=fake_key,
|
||||
root_helper=self.root_helper, run_as_root=True,
|
||||
check_exit_code=True),
|
||||
mock.call('cryptsetup', 'create', '--key-file=-', self.dev_name,
|
||||
self.dev_path, process_input=fake_key_mangled,
|
||||
root_helper=self.root_helper, run_as_root=True,
|
||||
check_exit_code=True),
|
||||
mock.call('ln', '--symbolic', '--force',
|
||||
'/dev/mapper/%s' % self.dev_name, self.symlink_path,
|
||||
root_helper=self.root_helper, run_as_root=True,
|
||||
check_exit_code=True),
|
||||
])
|
||||
self.assertEqual(3, mock_execute.call_count)
|
||||
|
|
|
@ -13,10 +13,8 @@
|
|||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import binascii
|
||||
from unittest import mock
|
||||
|
||||
from castellan.common.objects import symmetric_key as key
|
||||
from oslo_concurrency import processutils as putils
|
||||
|
||||
from os_brick.encryptors import luks
|
||||
|
@ -184,78 +182,6 @@ class LuksEncryptorTestCase(test_cryptsetup.CryptsetupEncryptorTestCase):
|
|||
attempts=3, run_as_root=True, check_exit_code=[0, 4]),
|
||||
])
|
||||
|
||||
def test_get_mangled_passphrase(self):
|
||||
# Confirm that a mangled passphrase is provided as per bug#1633518
|
||||
unmangled_raw_key = bytes(binascii.unhexlify('0725230b'))
|
||||
symmetric_key = key.SymmetricKey('AES', len(unmangled_raw_key) * 8,
|
||||
unmangled_raw_key)
|
||||
unmangled_encoded_key = symmetric_key.get_encoded()
|
||||
self.assertEqual(self.encryptor._get_mangled_passphrase(
|
||||
unmangled_encoded_key), '72523b')
|
||||
|
||||
@mock.patch('os_brick.executor.Executor._execute')
|
||||
def test_attach_volume_unmangle_passphrase(self, mock_execute):
|
||||
fake_key = '0725230b'
|
||||
fake_key_mangled = '72523b'
|
||||
self.encryptor._get_key = mock.MagicMock()
|
||||
self.encryptor._get_key.return_value = \
|
||||
test_cryptsetup.fake__get_key(None, fake_key)
|
||||
|
||||
mock_execute.side_effect = [
|
||||
putils.ProcessExecutionError(exit_code=2), # luksOpen
|
||||
mock.DEFAULT, # luksOpen
|
||||
mock.DEFAULT, # luksClose
|
||||
mock.DEFAULT, # luksAddKey
|
||||
mock.DEFAULT, # luksOpen
|
||||
mock.DEFAULT, # luksClose
|
||||
mock.DEFAULT, # luksRemoveKey
|
||||
mock.DEFAULT, # luksOpen
|
||||
mock.DEFAULT, # ln
|
||||
]
|
||||
|
||||
self.encryptor.attach_volume(None)
|
||||
|
||||
mock_execute.assert_has_calls([
|
||||
mock.call('cryptsetup', 'luksOpen', '--key-file=-', self.dev_path,
|
||||
self.dev_name, process_input=fake_key,
|
||||
root_helper=self.root_helper, run_as_root=True,
|
||||
check_exit_code=True),
|
||||
mock.call('cryptsetup', 'luksOpen', '--key-file=-', self.dev_path,
|
||||
self.dev_name, process_input=fake_key_mangled,
|
||||
root_helper=self.root_helper, run_as_root=True,
|
||||
check_exit_code=True),
|
||||
mock.call('cryptsetup', 'luksClose', self.dev_name,
|
||||
root_helper=self.root_helper, run_as_root=True,
|
||||
check_exit_code=[0, 4], attempts=3),
|
||||
mock.call('cryptsetup', 'luksAddKey', self.dev_path,
|
||||
'--force-password',
|
||||
process_input=''.join([fake_key_mangled,
|
||||
'\n', fake_key,
|
||||
'\n', fake_key]),
|
||||
root_helper=self.root_helper, run_as_root=True,
|
||||
check_exit_code=True),
|
||||
mock.call('cryptsetup', 'luksOpen', '--key-file=-', self.dev_path,
|
||||
self.dev_name, process_input=fake_key,
|
||||
root_helper=self.root_helper, run_as_root=True,
|
||||
check_exit_code=True),
|
||||
mock.call('cryptsetup', 'luksClose', self.dev_name,
|
||||
root_helper=self.root_helper, run_as_root=True,
|
||||
check_exit_code=[0, 4], attempts=3),
|
||||
mock.call('cryptsetup', 'luksRemoveKey', self.dev_path,
|
||||
process_input=fake_key_mangled,
|
||||
root_helper=self.root_helper, run_as_root=True,
|
||||
check_exit_code=True),
|
||||
mock.call('cryptsetup', 'luksOpen', '--key-file=-', self.dev_path,
|
||||
self.dev_name, process_input=fake_key,
|
||||
root_helper=self.root_helper, run_as_root=True,
|
||||
check_exit_code=True),
|
||||
mock.call('ln', '--symbolic', '--force',
|
||||
'/dev/mapper/%s' % self.dev_name, self.symlink_path,
|
||||
root_helper=self.root_helper, run_as_root=True,
|
||||
check_exit_code=True),
|
||||
], any_order=False)
|
||||
self.assertEqual(9, mock_execute.call_count)
|
||||
|
||||
|
||||
class Luks2EncryptorTestCase(LuksEncryptorTestCase):
|
||||
def _create(self):
|
||||
|
|
|
@ -0,0 +1,8 @@
|
|||
---
|
||||
upgrade:
|
||||
- |
|
||||
A workaround for
|
||||
`Bug #1633518 <https://bugs.launchpad.net/cinder/+bug/1633518>`_,
|
||||
where mangled passwords were used for various encryptors, has been
|
||||
removed. This was first introduced way back in the 1.9.0 Ocata-era release
|
||||
and has had more than enough time to bed in.
|
Loading…
Reference in New Issue