From ba202f6a0c0d0d6112c71cc8811f87354813130c Mon Sep 17 00:00:00 2001 From: Silvan Kaiser Date: Wed, 14 Oct 2015 13:49:09 +0200 Subject: [PATCH] Support insecure NAS security options in Quobyte So far the nas_secure_file_* options have been hardcoded to 'true' in the Quobyte Cinder driver. This change implements tests and a change to the set_nas_security_options method that allows setting these options to false. Implements: blueprint allow-insecure-conf-in-quobyte Change-Id: Iffcafd843340059f9b35e9844939cdb004cafbea --- cinder/tests/unit/test_quobyte.py | 42 ++++++++++++++++++++++++- cinder/volume/drivers/quobyte.py | 52 ++++++++++++++++++++++++++----- 2 files changed, 86 insertions(+), 8 deletions(-) diff --git a/cinder/tests/unit/test_quobyte.py b/cinder/tests/unit/test_quobyte.py index ed91d4b3c..16afbbd94 100644 --- a/cinder/tests/unit/test_quobyte.py +++ b/cinder/tests/unit/test_quobyte.py @@ -86,6 +86,8 @@ class QuobyteDriverTestCase(test.TestCase): self._configuration.quobyte_qcow2_volumes = False self._configuration.quobyte_mount_point_base = \ self.TEST_MNT_POINT_BASE + self._configuration.nas_secure_file_operations = "auto" + self._configuration.nas_secure_file_permissions = "auto" self._driver =\ quobyte.QuobyteDriver(configuration=self._configuration, @@ -342,11 +344,15 @@ class QuobyteDriverTestCase(test.TestCase): self.assertEqual(1, len(drv.shares)) self.assertEqual(0, len(drv._mounted_shares)) - def test_do_setup(self): + @mock.patch.object(quobyte.QuobyteDriver, "set_nas_security_options") + def test_do_setup(self, qb_snso_mock): """do_setup runs successfully.""" drv = self._driver + drv.do_setup(mock.create_autospec(context.RequestContext)) + qb_snso_mock.assert_called_once_with(is_new_cinder_install=mock.ANY) + def test_check_for_setup_error_throws_quobyte_volume_url_not_set(self): """check_for_setup_error throws if 'quobyte_volume_url' is not set.""" drv = self._driver @@ -931,3 +937,37 @@ class QuobyteDriverTestCase(test.TestCase): mock_upload_volume.assert_called_once_with( mock.ANY, mock.ANY, mock.ANY, upload_path) self.assertTrue(mock_create_temporary_file.called) + + def test_set_nas_security_options_default(self): + drv = self._driver + self.assertTrue(drv.configuration.nas_secure_file_operations == + "true") + self.assertTrue(drv.configuration.nas_secure_file_permissions == + "true") + self.assertFalse(drv._execute_as_root) + + def test_set_nas_security_options_insecure(self): + drv = self._driver + drv.configuration.nas_secure_file_operations = "false" + drv.configuration.nas_secure_file_permissions = "false" + + drv.set_nas_security_options(is_new_cinder_install=True) + + self.assertTrue(drv.configuration.nas_secure_file_operations == + "false") + self.assertTrue(drv.configuration.nas_secure_file_permissions == + "false") + self.assertTrue(drv._execute_as_root) + + def test_set_nas_security_options_explicitly_secure(self): + drv = self._driver + drv.configuration.nas_secure_file_operations = "true" + drv.configuration.nas_secure_file_permissions = "true" + + drv.set_nas_security_options(is_new_cinder_install=True) + + self.assertTrue(drv.configuration.nas_secure_file_operations == + "true") + self.assertTrue(drv.configuration.nas_secure_file_permissions == + "true") + self.assertFalse(drv._execute_as_root) diff --git a/cinder/volume/drivers/quobyte.py b/cinder/volume/drivers/quobyte.py index 9f6559cf9..76a4559d9 100644 --- a/cinder/volume/drivers/quobyte.py +++ b/cinder/volume/drivers/quobyte.py @@ -29,7 +29,7 @@ from cinder.image import image_utils from cinder import utils from cinder.volume.drivers import remotefs as remotefs_drv -VERSION = '1.0' +VERSION = '1.1' LOG = logging.getLogger(__name__) @@ -78,6 +78,7 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriver): Version history: 1.0 - Initial driver. + 1.1 - Adds optional insecure NAS settings """ driver_volume_type = 'quobyte' @@ -94,9 +95,9 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriver): def do_setup(self, context): """Any initialization the volume driver does while starting.""" - self.set_nas_security_options(is_new_cinder_install=False) super(QuobyteDriver, self).do_setup(context) + self.set_nas_security_options(is_new_cinder_install=False) self.shares = {} # address : options self._nova = compute.API() @@ -120,10 +121,47 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriver): raise def set_nas_security_options(self, is_new_cinder_install): - self.configuration.nas_secure_file_operations = 'true' - self.configuration.nas_secure_file_permissions = 'true' self._execute_as_root = False + LOG.debug("nas_secure_file_* settings are %(ops)s and %(perm)s", + {'ops': self.configuration.nas_secure_file_operations, + 'perm': self.configuration.nas_secure_file_permissions} + ) + + if self.configuration.nas_secure_file_operations == 'auto': + """Note (kaisers): All previous Quobyte driver versions ran with + secure settings hardcoded to 'True'. Therefore the default 'auto' + setting can safely be mapped to the same, secure, setting. + """ + LOG.debug("Mapping 'auto' value to 'true' for" + " nas_secure_file_operations.") + self.configuration.nas_secure_file_operations = 'true' + + if self.configuration.nas_secure_file_permissions == 'auto': + """Note (kaisers): All previous Quobyte driver versions ran with + secure settings hardcoded to 'True'. Therefore the default 'auto' + setting can safely be mapped to the same, secure, setting. + """ + LOG.debug("Mapping 'auto' value to 'true' for" + " nas_secure_file_permissions.") + self.configuration.nas_secure_file_permissions = 'true' + + if self.configuration.nas_secure_file_operations == 'false': + LOG.warning(_LW("The NAS file operations will be run as " + "root, allowing root level access at the storage " + "backend.")) + self._execute_as_root = True + else: + LOG.info(_LI("The NAS file operations will be run as" + " non privileged user in secure mode. Please" + " ensure your libvirtd settings have been configured" + " accordingly (see section 'OpenStack' in the Quobyte" + " Manual.")) + + if self.configuration.nas_secure_file_permissions == 'false': + LOG.warning(_LW("The NAS file permissions mode will be 666 " + "(allowing other/world read & write access).")) + def _qemu_img_info(self, path, volume_name): return super(QuobyteDriver, self)._qemu_img_info_base( path, volume_name, self.configuration.quobyte_mount_point_base) @@ -391,7 +429,7 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriver): LOG.info(_LI('Fixing previous mount %s which was not' ' unmounted correctly.'), mount_path) self._execute('umount.quobyte', mount_path, - run_as_root=False) + run_as_root=self._execute_as_root) except processutils.ProcessExecutionError as exc: LOG.warning(_LW("Failed to unmount previous mount: " "%s"), exc) @@ -411,7 +449,7 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriver): try: LOG.info(_LI('Mounting volume: %s ...'), quobyte_volume) - self._execute(*command, run_as_root=False) + self._execute(*command, run_as_root=self._execute_as_root) LOG.info(_LI('Mounting volume: %s succeeded'), quobyte_volume) mounted = True except processutils.ProcessExecutionError as exc: @@ -427,7 +465,7 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriver): """Wraps execute calls for checking validity of a Quobyte volume""" command = ['getfattr', "-n", "quobyte.info", mount_path] try: - self._execute(*command, run_as_root=False) + self._execute(*command, run_as_root=self._execute_as_root) except processutils.ProcessExecutionError as exc: msg = (_("The mount %(mount_path)s is not a valid" " Quobyte USP volume. Error: %(exc)s")