RBD: cinderlib support for rbd_keyring_conf option

In the last cycle we deprecated the RBD configuration option as per
OSSN-0085, and it was removed for victoria by Change
I3cc58b2d74d82ab6b2186e9ea7cfafeb4c3de822

This patch modifies the RBD driver to support cinderlib use cases that
are not affected by the security vulnerability.

Even if we have the configuration option in cinder.conf it will not be
seen by the Cinder RBD driver, it will only see it if we skip the Oslo
Config mechanism and set it directly on the instance as an attribute,
like cinderlib does.

Related-Bug: #1849624
Implements: blueprint rbd-remove-option-causing-ossn-0085
Change-Id: Iae63aee973932b2eef26d832a7f413a04bad0df1

cinder/tests/unit/volume/drivers/test_rbd.py

@@ -258,6 +258,16 @@ class RBDTestCase(test.TestCase):
             self.assertRaises(exception.InvalidConfigurationValue,
                               self.driver.check_for_setup_error)
 
+    @mock.patch.object(driver, 'rados', mock.Mock())
+    @mock.patch.object(driver, 'RADOSClient')
+    def test_check_for_setup_error_missing_keyring_data(self, mock_client):
+        self.driver.keyring_file = '/etc/ceph/ceph.client.admin.keyring'
+        self.driver.keyring_data = None
+
+        self.assertRaises(exception.InvalidConfigurationValue,
+                          self.driver.check_for_setup_error)
+        mock_client.assert_called_once_with(self.driver)
+
     def test_parse_replication_config_empty(self):
         self.driver._parse_replication_configs([])
         self.assertEqual([], self.driver._replication_targets)
@@ -1593,6 +1603,12 @@ class RBDTestCase(test.TestCase):
         }
         self._initialize_connection_helper(expected, hosts, ports)
 
+        # Check how it will work with keyring data (for cinderlib)
+        keyring_data = "[client.cinder]\n  key = test\n"
+        self.driver.keyring_data = keyring_data
+        expected['data']['keyring'] = keyring_data
+        self._initialize_connection_helper(expected, hosts, ports)
+
         self.driver._active_config = {'name': 'secondary_id',
                                       'user': 'foo',
                                       'conf': 'bar',
@@ -1600,6 +1616,68 @@ class RBDTestCase(test.TestCase):
         expected['data']['secret_uuid'] = 'secondary_secret_uuid'
         self._initialize_connection_helper(expected, hosts, ports)
 
+    def test__set_keyring_attributes_openstack(self):
+        # OpenStack usage doesn't have the rbd_keyring_conf Oslo Config option
+        self.assertFalse(hasattr(self.driver.configuration,
+                                 'rbd_keyring_conf'))
+        # Set initial values so we can confirm that we set them to None
+        self.driver.keyring_file = mock.sentinel.keyring_file
+        self.driver.keyring_data = mock.sentinel.keyring_data
+
+        self.driver._set_keyring_attributes()
+
+        self.assertIsNone(self.driver.keyring_file)
+        self.assertIsNone(self.driver.keyring_data)
+
+    def test__set_keyring_attributes_cinderlib(self):
+        # OpenStack usage doesn't have the rbd_keyring_conf Oslo Config option
+        cfg_file = '/etc/ceph/ceph.client.admin.keyring'
+        self.driver.configuration.rbd_keyring_conf = cfg_file
+        self.driver._set_keyring_attributes()
+        self.assertEqual(cfg_file, self.driver.keyring_file)
+        self.assertIsNone(self.driver.keyring_data)
+
+    @mock.patch('os.path.isfile')
+    @mock.patch.object(driver, 'open')
+    def test__set_keyring_attributes_cinderlib_read_file(self, mock_open,
+                                                         mock_isfile):
+        cfg_file = '/etc/ceph/ceph.client.admin.keyring'
+        # This is how cinderlib sets the config option
+        setattr(self.driver.configuration, 'rbd_keyring_conf', cfg_file)
+
+        keyring_data = "[client.cinder]\n  key = test\n"
+        mock_read = mock_open.return_value.__enter__.return_value.read
+        mock_read.return_value = keyring_data
+
+        self.assertIsNone(self.driver.keyring_file)
+        self.assertIsNone(self.driver.keyring_data)
+
+        self.driver._set_keyring_attributes()
+
+        mock_isfile.assert_called_once_with(cfg_file)
+        mock_open.assert_called_once_with(cfg_file, 'r')
+        mock_read.assert_called_once_with()
+        self.assertEqual(cfg_file, self.driver.keyring_file)
+        self.assertEqual(keyring_data, self.driver.keyring_data)
+
+    @mock.patch('os.path.isfile')
+    @mock.patch.object(driver, 'open', side_effect=IOError)
+    def test__set_keyring_attributes_cinderlib_error(self, mock_open,
+                                                     mock_isfile):
+        cfg_file = '/etc/ceph/ceph.client.admin.keyring'
+        # This is how cinderlib sets the config option
+        setattr(self.driver.configuration, 'rbd_keyring_conf', cfg_file)
+
+        self.assertIsNone(self.driver.keyring_file)
+        self.driver.keyring_data = mock.sentinel.keyring_data
+
+        self.driver._set_keyring_attributes()
+
+        mock_isfile.assert_called_once_with(cfg_file)
+        mock_open.assert_called_once_with(cfg_file, 'r')
+        self.assertEqual(cfg_file, self.driver.keyring_file)
+        self.assertIsNone(self.driver.keyring_data)
+
     @ddt.data({'rbd_chunk_size': 1, 'order': 20},
               {'rbd_chunk_size': 8, 'order': 23},
               {'rbd_chunk_size': 32, 'order': 25})

cinder/volume/drivers/rbd.py

@@ -261,6 +261,30 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
             self.RBD_FEATURE_OBJECT_MAP |
             self.RBD_FEATURE_EXCLUSIVE_LOCK)
 
+        self._set_keyring_attributes()
+
+    def _set_keyring_attributes(self):
+        # The rbd_keyring_conf option is not available for OpenStack usage
+        # for security reasons (OSSN-0085) and in OpenStack we use
+        # rbd_secret_uuid or make sure that the keyring files are present on
+        # the hosts (where os-brick will look for them).
+        # For cinderlib usage this option is necessary (no security issue, as
+        # in those cases the contents of the connection are not available to
+        # users). By using getattr Oslo-conf won't read the option from the
+        # file even if it's there (because we have removed the conf option
+        # definition), but cinderlib will find it because it sets the option
+        # directly as an attribute.
+        self.keyring_file = getattr(self.configuration, 'rbd_keyring_conf',
+                                    None)
+
+        self.keyring_data = None
+        try:
+            if self.keyring_file and os.path.isfile(self.keyring_file):
+                with open(self.keyring_file, 'r') as k_file:
+                    self.keyring_data = k_file.read()
+        except IOError:
+            LOG.debug('Cannot read RBD keyring file: %s.', self.keyring_file)
+
     @classmethod
     def get_driver_options(cls):
         additional_opts = cls._get_oslo_driver_opts(
@@ -388,6 +412,14 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
                 LOG.error(msg)
                 raise exception.VolumeBackendAPIException(data=msg)
 
+        # If the keyring is defined (cinderlib usage), then the contents are
+        # necessary.
+        if self.keyring_file and not self.keyring_data:
+            msg = _('No keyring data found')
+            LOG.error(msg)
+            raise exception.InvalidConfigurationValue(
+                option='rbd_keyring_conf', value=self.keyring_file)
+
         self._start_periodic_tasks()
 
     def RBDProxy(self):
@@ -1423,6 +1455,8 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
                 "discard": True,
             }
         }
+        if self.keyring_data:
+            data['data']['keyring'] = self.keyring_data
         LOG.debug('connection data: %s', data)
         return data