From f0cef07bef5ea8ed29179ee3774df5f4a634ba86 Mon Sep 17 00:00:00 2001 From: Eric Young Date: Thu, 22 Mar 2018 20:24:01 -0400 Subject: [PATCH] ScaleIO: Prevent usage of unsafe volumes It is possible for volumes, created from storage pools which have zero-padding disabled, to contain previous data. This change prevents these volumes from being created by default. A user can override this behavior by acknowleding the possibility with a configuration option. This is a squash of the four commits that led to the final state in rocky to not allow the creation of any type of non-zero-padded volumes to be created. This adds a config option that defaults to the safe behavior. It is backporting a new config option, and a change in default behavior, but it should be acceptable in this case so that the security vulnerability can be addressed. Closes-Bug: #1784871 Change-Id: I62f8f48b1624fc9abb7427bd4ca51f7873d35b96 Closes-bug: #1699573 (cherry picked from commit 7feb62197d371ab7253dc86a34af6ff8b484b4df) (cherry picked from commit 949cc46e162e00092aa85a7be921649c8dbf2bf8) (cherry picked from commit 8d0dea694a366cb3797748d389ca76b7864af16f) (cherry picked from commit 13a6689ccb7751c9f9b5c37ce0a3f75eb7665a95) --- .../volume/drivers/dell_emc/scaleio/mocks.py | 3 ++ .../volume/drivers/dell_emc/scaleio/driver.py | 52 ++++++++++++++++++- .../scaleio-zeropadding-a0273c56c4d14fca.yaml | 8 +++ 3 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/scaleio-zeropadding-a0273c56c4d14fca.yaml diff --git a/cinder/tests/unit/volume/drivers/dell_emc/scaleio/mocks.py b/cinder/tests/unit/volume/drivers/dell_emc/scaleio/mocks.py index b0e51363a47..fa77cb3de3b 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/scaleio/mocks.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/scaleio/mocks.py @@ -40,6 +40,9 @@ class ScaleIODriver(driver.ScaleIODriver): def unmanage(self, volume): pass + def _is_volume_creation_safe(self, _pd, _sp): + return True + class MockHTTPSResponse(requests.Response): """Mock HTTP Response diff --git a/cinder/volume/drivers/dell_emc/scaleio/driver.py b/cinder/volume/drivers/dell_emc/scaleio/driver.py index 3b9281ac32a..12c92b2845c 100644 --- a/cinder/volume/drivers/dell_emc/scaleio/driver.py +++ b/cinder/volume/drivers/dell_emc/scaleio/driver.py @@ -101,7 +101,13 @@ scaleio_opts = [ 'driver. This replaces the general ' 'max_over_subscription_ratio which has no effect ' 'in this driver.' - 'Maximum value allowed for ScaleIO is 10.0.') + 'Maximum value allowed for ScaleIO is 10.0.'), + cfg.BoolOpt('sio_allow_non_padded_volumes', + default=False, + help='Allow volumes to be created in Storage Pools ' + 'when zero padding is disabled. This option should ' + 'not be enabled if multiple tenants will utilize ' + 'volumes from a shared Storage Pool.'), ] CONF.register_opts(scaleio_opts, group=configuration.SHARED_CONF_GROUP) @@ -481,6 +487,34 @@ class ScaleIODriver(driver.VolumeDriver): {'id': id, 'name': encoded_name}) return encoded_name + def _is_volume_creation_safe(self, + protection_domain, + storage_pool): + """Checks if volume creation is safe or not. + + Using volumes with zero padding disabled can lead to existing data + being read off of a newly created volume. + """ + # if we have been told to allow unsafe volumes + if self.configuration.sio_allow_non_padded_volumes: + # Enabled regardless of type, so safe to proceed + return True + + try: + properties = self._get_storage_pool_properties(protection_domain, + storage_pool) + padded = properties['zeroPaddingEnabled'] + except Exception: + msg = (_("Unable to retrieve properties for pool, %(pool)s") % + {'pool': storage_pool}) + raise exception.InvalidInput(reason=msg) + + # zero padded storage pools are safe + if padded: + return True + # if we got here, it's unsafe + return False + def create_volume(self, volume): """Creates a scaleIO volume.""" self._check_volume_size(volume.size) @@ -560,6 +594,22 @@ class ScaleIODriver(driver.VolumeDriver): else: provisioning = "ThickProvisioned" + allowed = self._is_volume_creation_safe(protection_domain_name, + storage_pool_name) + if not allowed: + # Do not allow thick volume creation on this backend. + # Volumes may leak data between tenants. + LOG.error("Volume creation rejected due to " + "zero padding being disabled for pool, %s:%s. " + "This behaviour can be changed by setting " + "the configuration option " + "sio_allow_non_padded_volumes = True.", + protection_domain_name, + storage_pool_name) + msg = _("Volume creation rejected due to " + "unsafe backend configuration.") + raise exception.VolumeBackendAPIException(data=msg) + # units.Mi = 1024 ** 2 volume_size_kb = volume.size * units.Mi params = {'protectionDomainId': domain_id, diff --git a/releasenotes/notes/scaleio-zeropadding-a0273c56c4d14fca.yaml b/releasenotes/notes/scaleio-zeropadding-a0273c56c4d14fca.yaml new file mode 100644 index 00000000000..18f3c42ee84 --- /dev/null +++ b/releasenotes/notes/scaleio-zeropadding-a0273c56c4d14fca.yaml @@ -0,0 +1,8 @@ +--- +security: + - | + Removed the ability to create volumes in a ScaleIO Storage Pool that has + zero-padding disabled. A new configuration option + ``sio_allow_non_padded_volumes`` has been added to override this new + behavior and allow unpadded volumes, but should not be enabled if multiple + tenants will utilize volumes from a shared Storage Pool.