Merge "encryptors: Remove workaround for bug #1633518"

This commit is contained in:
Zuul 2021-08-03 15:51:00 +00:00 committed by Gerrit Code Review
commit 72f7f5a614
5 changed files with 9 additions and 179 deletions

View File

@ -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',

View File

@ -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

View File

@ -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)

View File

@ -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):

View File

@ -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.