From 6309c097e653c5f8b40e0602950d0ef54a9efb37 Mon Sep 17 00:00:00 2001
From: Eric Young <eric.young@dell.com>
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)
---
 .../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 dd3911ccebf..b0f06bf3e2c 100644
--- a/cinder/tests/unit/volume/drivers/dell_emc/scaleio/mocks.py
+++ b/cinder/tests/unit/volume/drivers/dell_emc/scaleio/mocks.py
@@ -43,6 +43,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 278a95bc3db..1c995fe6885 100644
--- a/cinder/volume/drivers/dell_emc/scaleio/driver.py
+++ b/cinder/volume/drivers/dell_emc/scaleio/driver.py
@@ -99,7 +99,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)
@@ -446,6 +452,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)
@@ -525,6 +559,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.