openstack/manila
Code Issues Proposed changes

Only allow IP access type for CephFS NFS

Browse Source 
For the CephFS NFS back end only ``IP`` access type
is valid so enforce this in the driver.

Also validate access level since there is a utility
routine to check both access type and access level.

Closes-bug: #1816420
Change-Id: I6c96f861b30ef7ccac05a7c199a62f0d69044c3a
(cherry picked from commit 9df750fd19)
This commit is contained in:
Tom Barron 2019-02-28 15:35:12 -05:00 committed by Goutham Pacha Ravi
parent c9a0388e03
commit 08148bba49
5 changed files with 114 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

75
manila/share/drivers/ganesha/__init__.py
View File

@ -38,7 +38,7 @@ class NASHelperBase(object):
# drivers that use a helper derived from this class
# should pass the following attributes to
# ganesha_utils.validate_acces_rule in their
# ganesha_utils.validate_access_rule in their
# update_access implementation.
supported_access_types = ()
supported_access_levels = ()
@ -60,7 +60,8 @@ class GaneshaNASHelper(NASHelperBase):
"""Perform share access changes using Ganesha version < 2.4."""
supported_access_types = ('ip', )
supported_access_levels = (constants.ACCESS_LEVEL_RW, )
supported_access_levels = (constants.ACCESS_LEVEL_RW,
constants.ACCESS_LEVEL_RO)
def __init__(self, execute, config, tag='<no name>', **kwargs):
super(GaneshaNASHelper, self).__init__(execute, config, **kwargs)
@ -127,8 +128,9 @@ class GaneshaNASHelper(NASHelperBase):
def _allow_access(self, base_path, share, access):
"""Allow access to the share."""
if access['access_type'] != 'ip':
raise exception.InvalidShareAccess('Only IP access type allowed')
ganesha_utils.validate_access_rule(
self.supported_access_types, self.supported_access_levels,
access, abort=True)
access = ganesha_utils.fixup_access_rule(access)
@ -157,15 +159,23 @@ class GaneshaNASHelper(NASHelperBase):
def update_access(self, context, share, access_rules, add_rules,
delete_rules, share_server=None):
"""Update access rules of share."""
rule_state_map = {}
if not (add_rules or delete_rules):
add_rules = access_rules
self.ganesha.reset_exports()
self.ganesha.restart_service()
for rule in add_rules:
self._allow_access('/', share, rule)
try:
self._allow_access('/', share, rule)
except (exception.InvalidShareAccess,
exception.InvalidShareAccessLevel):
rule_state_map[rule['id']] = {'state': 'error'}
continue
for rule in delete_rules:
self._deny_access('/', share, rule)
return rule_state_map
class GaneshaNASHelper2(GaneshaNASHelper):
@ -228,6 +238,7 @@ class GaneshaNASHelper2(GaneshaNASHelper):
confdict = {}
existing_access_rules = []
rule_state_map = {}
if self.ganesha.check_export_exists(share['name']):
confdict = self.ganesha._read_export(share['name'])
@ -243,6 +254,16 @@ class GaneshaNASHelper2(GaneshaNASHelper):
wanted_rw_clients, wanted_ro_clients = [], []
for rule in access_rules:
try:
ganesha_utils.validate_access_rule(
self.supported_access_types, self.supported_access_levels,
rule, True)
except (exception.InvalidShareAccess,
exception.InvalidShareAccessLevel):
rule_state_map[rule['id']] = {'state': 'error'}
continue
rule = ganesha_utils.fixup_access_rule(rule)
if rule['access_level'] == 'rw':
wanted_rw_clients.append(rule['access_to'])
@ -263,28 +284,30 @@ class GaneshaNASHelper2(GaneshaNASHelper):
'Clients': ','.join(wanted_rw_clients)
})
if existing_access_rules:
# Update existing export.
ganesha_utils.patch(confdict, {
'EXPORT': {
'CLIENT': clients
}
})
self.ganesha.update_export(share['name'], confdict)
else:
# Add new export.
ganesha_utils.patch(confdict, self.export_template, {
'EXPORT': {
'Export_Id': self.ganesha.get_export_id(),
'Path': self._get_export_path(share),
'Pseudo': self._get_export_pseudo_path(share),
'Tag': share['name'],
'CLIENT': clients,
'FSAL': self._fsal_hook(None, share, None)
}
})
self.ganesha.add_export(share['name'], confdict)
if clients: # Empty list if no rules passed validation
if existing_access_rules:
# Update existing export.
ganesha_utils.patch(confdict, {
'EXPORT': {
'CLIENT': clients
}
})
self.ganesha.update_export(share['name'], confdict)
else:
# Add new export.
ganesha_utils.patch(confdict, self.export_template, {
'EXPORT': {
'Export_Id': self.ganesha.get_export_id(),
'Path': self._get_export_path(share),
'Pseudo': self._get_export_pseudo_path(share),
'Tag': share['name'],
'CLIENT': clients,
'FSAL': self._fsal_hook(None, share, None)
}
})
self.ganesha.add_export(share['name'], confdict)
else:
# No clients have access to the share. Remove export.
self.ganesha.remove_export(share['name'])
self._cleanup_fsal_hook(None, share, None)
return rule_state_map

2
manila/share/drivers/ganesha/utils.py
View File

@ -139,7 +139,7 @@ def validate_access_rule(supported_access_types, supported_access_levels,
def fixup_access_rule(access_rule):
"""Adjust access rule as required for ganesha to handle it properly.
:param access_rule: Access rules to be validated.
:param access_rule: Access rules to be fixed up.
:return: access_rule
"""
if access_rule['access_to'] == '0.0.0.0/0':

3
manila/tests/db/fakes.py
View File

@ -42,6 +42,9 @@ class FakeModel(object):
def __contains__(self, key):
return self._getattr__(key)
def to_dict(self):
return self.values
def stub_out(stubs, funcs):
"""Set the stubs in mapping in the db api."""

53
manila/tests/share/drivers/test_ganesha.py
View File

@ -238,6 +238,8 @@ class GaneshaNASHelperTestCase(test.TestCase):
mock.Mock(return_value='fakefsal'))
self.mock_object(ganesha.ganesha_utils, 'patch',
mock.Mock(side_effect=fake_patch_run))
self.mock_object(ganesha.ganesha_utils, 'validate_access_rule',
mock.Mock(return_value=True))
ret = self._helper._allow_access(fake_basepath, self.share,
self.access)
self._helper.ganesha.get_export_id.assert_called_once_with()
@ -308,6 +310,26 @@ class GaneshaNASHelperTestCase(test.TestCase):
self.assertTrue(self._helper.ganesha.reset_exports.called)
self.assertTrue(self._helper.ganesha.restart_service.called)
def test_update_access_invalid_share_access_type(self):
bad_rule = fake_share.fake_access(access_type='notip', id='fakeid')
expected = {'fakeid': {'state': 'error'}}
result = self._helper.update_access(self._context, self.share,
access_rules=[bad_rule],
add_rules=[], delete_rules=[])
self.assertEqual(expected, result)
def test_update_access_invalid_share_access_level(self):
bad_rule = fake_share.fake_access(access_level='RO', id='fakeid')
expected = {'fakeid': {'state': 'error'}}
result = self._helper.update_access(self._context, self.share,
access_rules=[bad_rule],
add_rules=[], delete_rules=[])
self.assertEqual(expected, result)
@ddt.ddt
class GaneshaNASHelper2TestCase(test.TestCase):
@ -445,6 +467,8 @@ class GaneshaNASHelper2TestCase(test.TestCase):
mock.Mock(return_value='/fakepath'))
self.mock_object(self._helper, '_fsal_hook',
mock.Mock(return_value={'Name': 'fake'}))
self.mock_object(ganesha.ganesha_utils, 'validate_access_rule',
mock.Mock(return_value=True))
result_confdict = {
'EXPORT': {
'Export_Id': 100,
@ -484,6 +508,8 @@ class GaneshaNASHelper2TestCase(test.TestCase):
mock_gh, '_read_export',
mock.Mock(return_value={'EXPORT': {'CLIENT': client}})
)
self.mock_object(ganesha.ganesha_utils, 'validate_access_rule',
mock.Mock(return_value=True))
result_confdict = {
'EXPORT': {
'CLIENT': [
@ -541,3 +567,30 @@ class GaneshaNASHelper2TestCase(test.TestCase):
self.assertFalse(mock_gh.update_export.called)
self.assertFalse(mock_gh.remove_export.called)
self.assertFalse(self._helper._cleanup_fsal_hook.called)
def test_update_access_invalid_share_access_type(self):
mock_gh = self._helper.ganesha
self.mock_object(mock_gh, 'check_export_exists',
mock.Mock(return_value=False))
bad_rule = fake_share.fake_access(access_type='notip', id='fakeid')
expected = {'fakeid': {'state': 'error'}}
result = self._helper.update_access(self._context, self.share,
access_rules=[bad_rule],
add_rules=[], delete_rules=[])
self.assertEqual(expected, result)
def test_update_access_invalid_share_access_level(self):
bad_rule = fake_share.fake_access(access_level='NOT_RO_OR_RW',
id='fakeid')
expected = {'fakeid': {'state': 'error'}}
mock_gh = self._helper.ganesha
self.mock_object(mock_gh, 'check_export_exists',
mock.Mock(return_value=False))
result = self._helper.update_access(self._context, self.share,
access_rules=[bad_rule],
add_rules=[], delete_rules=[])
self.assertEqual(expected, result)

8
releasenotes/notes/bug-1816420-validate-access-type-for-ganehas-c42ce6f859fa0c8c.yaml Normal file
View File

@ -0,0 +1,8 @@
---
fixes:
- |
Access rule type for shares served via nfs-ganesha is now validated,
fixing `launchpad bug #1816420 <https://bugs.launchpad.net/manila/+bug/1811680>`_
where ``cephx`` access type was allowed though only ``ip`` access type
is effective. This fix also validates ``access_level`` to ensure that
it is set to ``RW`` or ``RO``.