From 234aab19f1677a337abd4cc37ede5dc5e455f258 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 f0cef07bef5ea8ed29179ee3774df5f4a634ba86) (cherry picked from commit 6309c097e653c5f8b40e0602950d0ef54a9efb37) (cherry picked from commit 2dc52153215bb6a37532a959c5c98239be21bb56) --- .../unit/volume/drivers/emc/scaleio/mocks.py | 3 ++ cinder/volume/drivers/emc/scaleio.py | 53 ++++++++++++++++++- .../scaleio-zeropadding-a0273c56c4d14fca.yaml | 8 +++ 3 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/scaleio-zeropadding-a0273c56c4d14fca.yaml diff --git a/cinder/tests/unit/volume/drivers/emc/scaleio/mocks.py b/cinder/tests/unit/volume/drivers/emc/scaleio/mocks.py index 4dee67db121..9372d6d516f 100644 --- a/cinder/tests/unit/volume/drivers/emc/scaleio/mocks.py +++ b/cinder/tests/unit/volume/drivers/emc/scaleio/mocks.py @@ -70,6 +70,9 @@ class ScaleIODriver(scaleio.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/emc/scaleio.py b/cinder/volume/drivers/emc/scaleio.py index fba7945f82e..b3e18aea7e4 100644 --- a/cinder/volume/drivers/emc/scaleio.py +++ b/cinder/volume/drivers/emc/scaleio.py @@ -77,7 +77,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) @@ -322,6 +328,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) @@ -445,6 +479,23 @@ 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(_LE("Volume creation rejected due to " + "zero padding being disabled for pool, " + "%(dom)s:%(pool)s. " + "This behaviour can be changed by setting " + "the configuration option " + "sio_allow_non_padded_volumes = True."), + {'dom': protection_domain_name, + 'pool': 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.