openstack/cinder
Code Issues Proposed changes

IBMNAS: Remove call to set r/w permissions to all

Browse Source 
During cinder volume create operation from a volume snapshot or
from an existing volume (volume clone operation), the ibmnas
driver sets 'rw' permissions to all, which is unnecessary and
also poses security concerns.
Fixing this issue, removing the calls to set rw permissions to
all during these operations and adding a call to set 'rw'
permissions only to the owner to make sure even if umask is set
at the filesystem level, which might deny 'rw' access to the owner
we explicitely set the required permissions on the volume file.

Change-Id: I0e5ba9262a298e088f7724ddeda3537afa4b023e
Closes-Bug: #1367238
This commit is contained in:
Nilesh Bhosale 2014-08-24 13:06:29 +05:30 committed by Jay S. Bryant
parent 58eda5d1f4
commit 3fff556817
3 changed files with 36 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
cinder/tests/test_ibmnas.py
View File

@ -196,7 +196,7 @@ class IBMNASDriverTestCase(test.TestCase):
@mock.patch('cinder.volume.drivers.ibm.ibmnas.IBMNAS_NFSDriver.'
'_ssh_operation')
@mock.patch('cinder.openstack.common.processutils.execute')
def test_create_ibmnas_snap_nas_gpfs(self, mock_ssh, mock_execute):
def test_create_ibmnas_snap_nas_gpfs(self, mock_execute, mock_ssh):
"""Create ibmnas snap if mount point is provided."""
drv = self._driver
@ -291,8 +291,8 @@ class IBMNASDriverTestCase(test.TestCase):
"""Extend volume to greater size test case."""
drv = self._driver
mock_resize.return_value = True
mock_local.return_value = self.TEST_LOCAL_PATH
mock_resize.return_value = True
volume = FakeEnv()
volume['name'] = 'vol-123'
@ -301,7 +301,7 @@ class IBMNASDriverTestCase(test.TestCase):
@mock.patch('cinder.volume.drivers.ibm.ibmnas.IBMNAS_NFSDriver._run_ssh')
@mock.patch('cinder.openstack.common.processutils.execute')
def test_delete_snapfiles(self, mock_ssh, mock_execute):
def test_delete_snapfiles(self, mock_execute, mock_ssh):
"""Delete_snapfiles test case."""
drv = self._driver
@ -309,15 +309,15 @@ class IBMNASDriverTestCase(test.TestCase):
'File name\n yes 0 /ibm/gpfs0/gshare/\n'
'volume-123\n EFSSG1000I The command'
'completed successfully.', '')
mock_execute.return_value = expected
mock_ssh.return_value = expected
mock_execute.return_value = expected
drv._delete_snapfiles(self.TEST_VOLUME_PATH,
self.TEST_MNT_POINT)
@mock.patch('cinder.volume.drivers.ibm.ibmnas.IBMNAS_NFSDriver._run_ssh')
@mock.patch('cinder.openstack.common.processutils.execute')
def test_delete_snapfiles_nas_gpfs(self, mock_ssh, mock_execute):
def test_delete_snapfiles_nas_gpfs(self, mock_execute, mock_ssh):
"""Delete_snapfiles for gpfs-nas platform test case."""
drv = self._driver
@ -328,8 +328,8 @@ class IBMNASDriverTestCase(test.TestCase):
'- ---------\n'
'yes 0\n'
'/ibm/gpfs0/gshare/volume-123', '')
mock_execute.return_value = expected
mock_ssh.return_value = expected
mock_execute.return_value = expected
drv._delete_snapfiles(self.TEST_VOLUME_PATH,
self.TEST_MNT_POINT)
@ -350,7 +350,7 @@ class IBMNASDriverTestCase(test.TestCase):
'_get_export_path')
@mock.patch('cinder.volume.drivers.ibm.ibmnas.IBMNAS_NFSDriver.'
'_delete_snapfiles')
def test_delete_volume(self, mock_export, mock_snap):
def test_delete_volume(self, mock_snap, mock_export):
"""Delete volume test case."""
drv = self._driver
@ -372,10 +372,8 @@ class IBMNASDriverTestCase(test.TestCase):
'_get_mount_point_for_share')
@mock.patch('cinder.volume.drivers.ibm.ibmnas.IBMNAS_NFSDriver.'
'_create_ibmnas_snap')
def test_create_snapshot(self, mock_export,
mock_provider,
mock_mount,
mock_snap):
def test_create_snapshot(self, mock_snap, mock_mount, mock_provider,
mock_export):
"""Create snapshot simple test case."""
drv = self._driver
@ -400,12 +398,12 @@ class IBMNASDriverTestCase(test.TestCase):
@mock.patch('cinder.volume.drivers.ibm.ibmnas.IBMNAS_NFSDriver.'
'_get_mount_point_for_share')
@mock.patch('cinder.openstack.common.processutils.execute')
def test_delete_snapshot(self, mock_mount, mock_provider, mock_execute):
def test_delete_snapshot(self, mock_execute, mock_mount, mock_provider):
"""Delete snapshot simple test case."""
drv = self._driver
mock_mount.return_value = self.TEST_LOCAL_PATH
mock_provider.return_value = self.TEST_VOLUME_PATH
mock_mount.return_value = self.TEST_LOCAL_PATH
mock_execute.return_value = True
volume = FakeEnv()
@ -424,20 +422,23 @@ class IBMNASDriverTestCase(test.TestCase):
@mock.patch('cinder.volume.drivers.ibm.ibmnas.IBMNAS_NFSDriver.'
'_create_ibmnas_copy')
@mock.patch('cinder.volume.drivers.ibm.ibmnas.IBMNAS_NFSDriver.'
'_resize_volume_file')
'_find_share')
@mock.patch('cinder.volume.drivers.ibm.ibmnas.IBMNAS_NFSDriver.local_path')
@mock.patch('cinder.volume.drivers.ibm.ibmnas.IBMNAS_NFSDriver.'
'_find_share')
'_set_rw_permissions_for_owner')
@mock.patch('cinder.volume.drivers.ibm.ibmnas.IBMNAS_NFSDriver.'
'_set_rw_permissions_for_all')
def test_create_cloned_volume(self, mock_export, mock_copy,
mock_resize, mock_local,
mock_find, mock_rw):
'_resize_volume_file')
def test_create_cloned_volume(self, mock_resize, mock_rw, mock_local,
mock_find, mock_copy, mock_export):
"""Clone volume with equal size test case."""
drv = self._driver
mock_export.return_value = self.TEST_VOLUME_PATH
mock_copy.return_value = self.TEST_LOCAL_PATH
mock_copy.return_value = True
mock_find.return_value = self.TEST_LOCAL_PATH
mock_local.return_value = self.TEST_LOCAL_PATH
mock_rw.return_value = True
mock_resize.return_value = True
volume_src = FakeEnv()
volume_src['id'] = '123'
@ -458,22 +459,24 @@ class IBMNASDriverTestCase(test.TestCase):
@mock.patch('cinder.volume.drivers.ibm.ibmnas.IBMNAS_NFSDriver.'
'_create_ibmnas_snap')
@mock.patch('cinder.volume.drivers.ibm.ibmnas.IBMNAS_NFSDriver.'
'_resize_volume_file')
'_find_share')
@mock.patch('cinder.volume.drivers.ibm.ibmnas.IBMNAS_NFSDriver.local_path')
@mock.patch('cinder.volume.drivers.ibm.ibmnas.IBMNAS_NFSDriver.'
'_find_share')
'_set_rw_permissions_for_owner')
@mock.patch('cinder.volume.drivers.ibm.ibmnas.IBMNAS_NFSDriver.'
'_set_rw_permissions_for_all')
def test_create_volume_from_snapshot(self, mock_export, mock_snap,
mock_resize, mock_local,
mock_find, mock_rw):
'_resize_volume_file')
def test_create_volume_from_snapshot(self, mock_resize, mock_rw,
mock_local, mock_find, mock_snap,
mock_export):
"""Create volume from snapshot test case."""
drv = self._driver
mock_export.return_value = '/export'
mock_snap.return_value = self.TEST_LOCAL_PATH
mock_local.return_value = self.TEST_VOLUME_PATH
mock_find.return_value = self.TEST_LOCAL_PATH
mock_local.return_value = self.TEST_VOLUME_PATH
mock_rw.return_value = True
mock_resize.return_value = True
volume = FakeEnv()
volume['id'] = '123'

5
cinder/volume/drivers/ibm/ibmnas.py
View File

@ -343,7 +343,7 @@ class IBMNAS_NFSDriver(nfs.NfsDriver, san.SanDriver):
volume['provider_location'] = self._find_share(volume['size'])
volume_path = self.local_path(volume)
self._set_rw_permissions_for_all(volume_path)
self._set_rw_permissions_for_owner(volume_path)
# Extend the volume if required
self._resize_volume_file(volume_path, volume['size'])
@ -365,8 +365,7 @@ class IBMNAS_NFSDriver(nfs.NfsDriver, san.SanDriver):
volume['provider_location'] = self._find_share(volume['size'])
volume_path = self.local_path(volume)
self._set_rw_permissions_for_all(volume_path)
self._set_rw_permissions_for_owner(volume_path)
# Extend the volume if required
self._resize_volume_file(volume_path, volume['size'])

4
cinder/volume/drivers/remotefs.py
View File

@ -229,6 +229,10 @@ class RemoteFSDriver(driver.VolumeDriver):
"""Sets 666 permissions for the path."""
self._execute('chmod', 'ugo+rw', path, run_as_root=True)
def _set_rw_permissions_for_owner(self, path):
"""Sets read-write permissions to the owner for the path."""
self._execute('chmod', 'u+rw', path, run_as_root=True)
def local_path(self, volume):
"""Get volume path (mounted locally fs path) for given volume
:param volume: volume reference