From 2aef694a87c6c038a50b2562a29aece3d7064f85 Mon Sep 17 00:00:00 2001 From: Brian Rosmaita Date: Fri, 21 Aug 2020 18:30:58 -0400 Subject: [PATCH] RBD: remove rbd_keyring_conf option This option was deprecated in the Ussuri release and is now removed. See OSSN-0085 for details. Change-Id: I3cc58b2d74d82ab6b2186e9ea7cfafeb4c3de822 Implements: bp rbd-remove-option-causing-ossn-0085 --- cinder/tests/unit/volume/drivers/test_rbd.py | 38 +------------------ cinder/volume/drivers/rbd.py | 22 ----------- ...ove-rbd_keyring_conf-2d54a4de634c255c.yaml | 9 +++++ 3 files changed, 10 insertions(+), 59 deletions(-) create mode 100644 releasenotes/notes/remove-rbd_keyring_conf-2d54a4de634c255c.yaml diff --git a/cinder/tests/unit/volume/drivers/test_rbd.py b/cinder/tests/unit/volume/drivers/test_rbd.py index 49109beda4e..6e8e65fe30d 100644 --- a/cinder/tests/unit/volume/drivers/test_rbd.py +++ b/cinder/tests/unit/volume/drivers/test_rbd.py @@ -171,7 +171,6 @@ class RBDTestCase(test.TestCase): cfg.rbd_cluster_name = 'nondefault' cfg.rbd_pool = 'rbd' cfg.rbd_ceph_conf = '/etc/ceph/my_ceph.conf' - cfg.rbd_keyring_conf = '/etc/ceph/my_ceph.client.keyring' cfg.rbd_secret_uuid = None cfg.rbd_user = 'cinder' cfg.volume_backend_name = None @@ -1569,15 +1568,10 @@ class RBDTestCase(test.TestCase): self.assertDictEqual(expected, actual) self.assertTrue(mock_get_mon_addrs.called) - @mock.patch.object(cinder.volume.drivers.rbd.RBDDriver, - '_get_keyring_contents') - def test_initialize_connection(self, mock_keyring): + def test_initialize_connection(self): hosts = ['::1', '::1', '::1', '127.0.0.1', 'example.com'] ports = ['6789', '6790', '6791', '6792', '6791'] - keyring_data = "[client.cinder]\n key = test\n" - mock_keyring.return_value = keyring_data - self.driver._active_config = {'name': 'secondary_id', 'user': 'foo', 'conf': 'bar'} @@ -1595,16 +1589,10 @@ class RBDTestCase(test.TestCase): 'secret_uuid': self.cfg.rbd_secret_uuid, 'volume_id': self.volume_a.id, 'discard': True, - 'keyring': keyring_data, } } self._initialize_connection_helper(expected, hosts, ports) - # Check how it will work with empty keyring path - mock_keyring.return_value = None - expected['data']['keyring'] = None - self._initialize_connection_helper(expected, hosts, ports) - self.driver._active_config = {'name': 'secondary_id', 'user': 'foo', 'conf': 'bar', @@ -1612,30 +1600,6 @@ class RBDTestCase(test.TestCase): expected['data']['secret_uuid'] = 'secondary_secret_uuid' self._initialize_connection_helper(expected, hosts, ports) - def test__get_keyring_contents_no_config_file(self): - self.cfg.rbd_keyring_conf = '' - self.assertIsNone(self.driver._get_keyring_contents()) - - @mock.patch('os.path.isfile') - def test__get_keyring_contents_read_file(self, mock_isfile): - mock_isfile.return_value = True - keyring_data = "[client.cinder]\n key = test\n" - mockopen = mock.mock_open(read_data=keyring_data) - mockopen.return_value.__exit__ = mock.Mock() - with mock.patch('cinder.volume.drivers.rbd.open', mockopen, - create=True): - self.assertEqual(self.driver._get_keyring_contents(), keyring_data) - - @mock.patch('os.path.isfile') - def test__get_keyring_contents_raise_error(self, mock_isfile): - mock_isfile.return_value = True - mockopen = mock.mock_open() - mockopen.return_value.__exit__ = mock.Mock() - with mock.patch('cinder.volume.drivers.rbd.open', mockopen, - create=True) as mock_keyring_file: - mock_keyring_file.side_effect = IOError - self.assertIsNone(self.driver._get_keyring_contents()) - @ddt.data({'rbd_chunk_size': 1, 'order': 20}, {'rbd_chunk_size': 8, 'order': 23}, {'rbd_chunk_size': 32, 'order': 25}) diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py index a5c901a2851..e5c3a89522f 100644 --- a/cinder/volume/drivers/rbd.py +++ b/cinder/volume/drivers/rbd.py @@ -65,13 +65,6 @@ RBD_OPTS = [ cfg.StrOpt('rbd_ceph_conf', default='', # default determined by librados help='Path to the ceph configuration file'), - cfg.StrOpt('rbd_keyring_conf', - deprecated_for_removal=True, - deprecated_reason='Use of this option exposes a security ' - 'vulnerability. See OSSN-0085 for details.', - deprecated_since='Ussuri', - default='', - help='Path to the ceph keyring file'), cfg.BoolOpt('rbd_flatten_volume_from_snapshot', default=False, help='Flatten volumes created from snapshots to remove ' @@ -1411,20 +1404,6 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, """Removes an export for a logical volume.""" pass - def _get_keyring_contents(self): - # NOTE(danpawlik) If keyring is not provided in Cinder configuration, - # os-brick library will take keyring from default path. - keyring_file = self.configuration.rbd_keyring_conf - keyring_data = None - try: - if os.path.isfile(keyring_file): - with open(keyring_file, 'r') as k_file: - keyring_data = k_file.read() - except IOError: - LOG.debug('Cannot read RBD keyring file: %s.', keyring_file) - - return keyring_data - def initialize_connection(self, volume, connector): hosts, ports = self._get_mon_addrs() name, conf, user, secret_uuid = self._get_config_tuple() @@ -1442,7 +1421,6 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, 'secret_uuid': secret_uuid, 'volume_id': volume.id, "discard": True, - 'keyring': self._get_keyring_contents(), } } LOG.debug('connection data: %s', data) diff --git a/releasenotes/notes/remove-rbd_keyring_conf-2d54a4de634c255c.yaml b/releasenotes/notes/remove-rbd_keyring_conf-2d54a4de634c255c.yaml new file mode 100644 index 00000000000..fa398314394 --- /dev/null +++ b/releasenotes/notes/remove-rbd_keyring_conf-2d54a4de634c255c.yaml @@ -0,0 +1,9 @@ +--- +upgrade: + - | + RBD driver: the ``rbd_keyring_conf`` configuration option, which + was deprecated in the Ussuri release, has been removed. If it is + present in a configuration file, its value will silently be + ignored. For more information, see `OSSN-0085 + `_: + Cinder configuration option can leak secret key from Ceph backend.