From fbd471cba7e60f740bf2ba86fe658ef8256915da Mon Sep 17 00:00:00 2001 From: Kaitlin Farr Date: Wed, 11 Nov 2015 13:32:44 -0500 Subject: [PATCH] Specify key algorithm and size for create_key Fixes a bug that was allowing the key manager to create the default size and algorithm of a key every time. This patch passes a key size and algorithm to the create_key command to provide more accurate information to the key manager. Another fix will be needed to address the use case of aes-xts with a key size of 512 -- key managers may not be able to create 512 bit AES keys. Change-Id: I166a7685914e25dff9268c86383ad68c41a21530 Closes-Bug: #1514546 --- cinder/tests/unit/keymgr/mock_key_mgr.py | 12 +++-- cinder/tests/unit/test_volume.py | 60 +++++++++++++++++++++++- cinder/volume/flows/api/create_volume.py | 14 +++++- 3 files changed, 78 insertions(+), 8 deletions(-) diff --git a/cinder/tests/unit/keymgr/mock_key_mgr.py b/cinder/tests/unit/keymgr/mock_key_mgr.py index 39f2caa6449..a5fcd9e04d3 100644 --- a/cinder/tests/unit/keymgr/mock_key_mgr.py +++ b/cinder/tests/unit/keymgr/mock_key_mgr.py @@ -52,17 +52,19 @@ class MockKeyManager(key_mgr.KeyManager): def __init__(self): self.keys = {} - def _generate_hex_key(self, **kwargs): - key_length = kwargs.get('key_length', 256) + def _generate_hex_key(self, length): + if not length: + length = 256 # hex digit => 4 bits - hex_encoded = utils.generate_password(length=key_length // 4, + hex_encoded = utils.generate_password(length=length // 4, symbolgroups='0123456789ABCDEF') return hex_encoded def _generate_key(self, **kwargs): - _hex = self._generate_hex_key(**kwargs) + _hex = self._generate_hex_key(kwargs.get('key_length')) key_bytes = array.array('B', binascii.unhexlify(_hex)).tolist() - return key.SymmetricKey('AES', key_bytes) + algorithm = kwargs.get('algorithm', 'AES') + return key.SymmetricKey(algorithm, key_bytes) def create_key(self, ctxt, **kwargs): """Creates a key. diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index 4d50a3f2795..ec7aa12b336 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -1047,16 +1047,23 @@ class VolumeTestCase(BaseVolumeTestCase): self.assertEqual(db_vol_type.get('id'), volume['volume_type_id']) @mock.patch.object(keymgr, 'API', fake_keymgr.fake_api) - def test_create_volume_with_encrypted_volume_type(self): + def test_create_volume_with_encrypted_volume_type_aes(self): ctxt = context.get_admin_context() + cipher = 'aes-xts-plain64' + key_size = 256 + control_location = 'front-end' + db.volume_type_create(ctxt, {'id': '61298380-0c12-11e3-bfd6-4b48424183be', 'name': 'LUKS'}) db.volume_type_encryption_create( ctxt, '61298380-0c12-11e3-bfd6-4b48424183be', - {'control_location': 'front-end', 'provider': ENCRYPTION_PROVIDER}) + {'control_location': control_location, + 'provider': ENCRYPTION_PROVIDER, + 'cipher': cipher, + 'key_size': key_size}) volume_api = cinder.volume.api.API() @@ -1067,7 +1074,56 @@ class VolumeTestCase(BaseVolumeTestCase): 'name', 'description', volume_type=db_vol_type) + + key_manager = volume_api.key_manager + key = key_manager.get_key(self.context, volume['encryption_key_id']) + self.assertEqual(key_size, len(key.key) * 8) + self.assertEqual('aes', key.alg) + + metadata = db.volume_encryption_metadata_get(self.context, volume.id) self.assertEqual(db_vol_type.get('id'), volume['volume_type_id']) + self.assertEqual(cipher, metadata.get('cipher')) + self.assertEqual(key_size, metadata.get('key_size')) + self.assertIsNotNone(volume['encryption_key_id']) + + @mock.patch.object(keymgr, 'API', fake_keymgr.fake_api) + def test_create_volume_with_encrypted_volume_type_blowfish(self): + ctxt = context.get_admin_context() + + cipher = 'blowfish-cbc' + key_size = 32 + control_location = 'front-end' + + db.volume_type_create(ctxt, + {'id': '61298380-0c12-11e3-bfd6-4b48424183be', + 'name': 'LUKS'}) + db.volume_type_encryption_create( + ctxt, + '61298380-0c12-11e3-bfd6-4b48424183be', + {'control_location': control_location, + 'provider': ENCRYPTION_PROVIDER, + 'cipher': cipher, + 'key_size': key_size}) + + volume_api = cinder.volume.api.API() + + db_vol_type = db.volume_type_get_by_name(ctxt, 'LUKS') + + volume = volume_api.create(self.context, + 1, + 'name', + 'description', + volume_type=db_vol_type) + + key_manager = volume_api.key_manager + key = key_manager.get_key(self.context, volume['encryption_key_id']) + self.assertEqual(key_size, len(key.key) * 8) + self.assertEqual('blowfish', key.alg) + + metadata = db.volume_encryption_metadata_get(self.context, volume.id) + self.assertEqual(db_vol_type.get('id'), volume['volume_type_id']) + self.assertEqual(cipher, metadata.get('cipher')) + self.assertEqual(key_size, metadata.get('key_size')) self.assertIsNotNone(volume['encryption_key_id']) def test_create_volume_with_provider_id(self): diff --git a/cinder/volume/flows/api/create_volume.py b/cinder/volume/flows/api/create_volume.py index 89dbbd645ed..ed4a095d5ec 100644 --- a/cinder/volume/flows/api/create_volume.py +++ b/cinder/volume/flows/api/create_volume.py @@ -357,7 +357,19 @@ class ExtractVolumeRequestTask(flow_utils.CinderTask): encryption_key_id = key_manager.copy_key(context, encryption_key_id) else: - encryption_key_id = key_manager.create_key(context) + volume_type_encryption = ( + volume_types.get_volume_type_encryption(context, + volume_type_id)) + cipher = volume_type_encryption.cipher + length = volume_type_encryption.key_size + + # NOTE(kaitlin-farr): dm-crypt expects the cipher in a + # hyphenated format (aes-xts-plain64). The algorithm needs + # to be parsed out to pass to the key manager (aes). + algorithm = cipher.split('-')[0] if cipher else None + encryption_key_id = key_manager.create_key(context, + algorithm=algorithm, + key_length=length) return encryption_key_id