From da3ab2cf4512716fa47a16315e98e610fbaed829 Mon Sep 17 00:00:00 2001 From: Goutham Pacha Ravi Date: Mon, 25 Jan 2021 23:44:32 -0800 Subject: [PATCH] [Native CephFS] Add messages for async ACL ops Access rules added to CephFS shares can fail at the driver, or by the ceph volume client library. Since the share manager can supply rule changes to the driver in batches, the driver has to gracefully handle individual rule failures. Further some of the causes of the access rule failures can be remedied by end users, therefore asynchronous user messages would be a good vehicle to register user faults that can be examined and corrected. Related-Bug: #1904015 [1] https://cve.mitre.org/cgi-bin/cvename.cgi?name=2020-27781 Change-Id: I3882fe5b1ad4a6cc71c13ea70fd6aea10430c42e Signed-off-by: Goutham Pacha Ravi --- manila/exception.py | 4 + manila/message/message_field.py | 73 ++++++++++------ manila/share/drivers/cephfs/driver.py | 66 +++++++++++--- .../tests/share/drivers/cephfs/test_driver.py | 86 ++++++++++++++++--- ...hx-asynchronous-msgs-6a683076a1fb5a54.yaml | 9 ++ 5 files changed, 187 insertions(+), 51 deletions(-) create mode 100644 releasenotes/notes/bug-1904015-cve-2020-27781-cephx-asynchronous-msgs-6a683076a1fb5a54.yaml diff --git a/manila/exception.py b/manila/exception.py index 509cbc890f..69a3099a50 100644 --- a/manila/exception.py +++ b/manila/exception.py @@ -520,6 +520,10 @@ class InvalidShareAccessLevel(Invalid): message = _("Invalid or unsupported share access level: %(level)s.") +class InvalidShareAccessType(Invalid): + message = _("Invalid or unsupported share access type: %(type)s.") + + class ShareBackendException(ManilaException): message = _("Share backend error: %(msg)s.") diff --git a/manila/message/message_field.py b/manila/message/message_field.py index 19d18e53e9..b96e1bfaf4 100644 --- a/manila/message/message_field.py +++ b/manila/message/message_field.py @@ -33,15 +33,19 @@ class Action(object): DELETE = ('007', _('delete')) EXTEND = ('008', _('extend')) SHRINK = ('009', _('shrink')) - ALL = (ALLOCATE_HOST, - CREATE, - DELETE_ACCESS_RULES, - PROMOTE, - UPDATE, - REVERT_TO_SNAPSHOT, - DELETE, - EXTEND, - SHRINK) + UPDATE_ACCESS_RULES = ('010', _('update access rules')) + ALL = ( + ALLOCATE_HOST, + CREATE, + DELETE_ACCESS_RULES, + PROMOTE, + UPDATE, + REVERT_TO_SNAPSHOT, + DELETE, + EXTEND, + SHRINK, + UPDATE_ACCESS_RULES, + ) class Detail(object): @@ -98,26 +102,39 @@ class Detail(object): '019', _("Share Driver does not support shrinking shares." " Shrinking share operation failed.")) + FORBIDDEN_CLIENT_ACCESS = ( + '020', + _("Failed to grant access to client. The client ID used may be " + "forbidden. You may try again with a different client identifier.")) + UNSUPPORTED_CLIENT_ACCESS = ( + '021', + _("Failed to grant access to client. The access level or type may " + "be unsupported. You may try again with a different access level " + "or access type.")) - ALL = (UNKNOWN_ERROR, - NO_VALID_HOST, - UNEXPECTED_NETWORK, - NO_SHARE_SERVER, - NO_ACTIVE_AVAILABLE_REPLICA, - NO_ACTIVE_REPLICA, - FILTER_AVAILABILITY, - FILTER_CAPABILITIES, - FILTER_CAPACITY, - FILTER_DRIVER, - FILTER_IGNORE, - FILTER_JSON, - FILTER_RETRY, - FILTER_REPLICATION, - DRIVER_FAILED_EXTEND, - FILTER_CREATE_FROM_SNAPSHOT, - DRIVER_FAILED_CREATING_FROM_SNAP, - DRIVER_REFUSED_SHRINK, - DRIVER_FAILED_SHRINK) + ALL = ( + UNKNOWN_ERROR, + NO_VALID_HOST, + UNEXPECTED_NETWORK, + NO_SHARE_SERVER, + NO_ACTIVE_AVAILABLE_REPLICA, + NO_ACTIVE_REPLICA, + FILTER_AVAILABILITY, + FILTER_CAPABILITIES, + FILTER_CAPACITY, + FILTER_DRIVER, + FILTER_IGNORE, + FILTER_JSON, + FILTER_RETRY, + FILTER_REPLICATION, + DRIVER_FAILED_EXTEND, + FILTER_CREATE_FROM_SNAPSHOT, + DRIVER_FAILED_CREATING_FROM_SNAP, + DRIVER_REFUSED_SHRINK, + DRIVER_FAILED_SHRINK, + FORBIDDEN_CLIENT_ACCESS, + UNSUPPORTED_CLIENT_ACCESS, + ) # Exception and detail mappings EXCEPTION_DETAIL_MAPPINGS = { diff --git a/manila/share/drivers/cephfs/driver.py b/manila/share/drivers/cephfs/driver.py index 42cfa38c47..83356d6763 100644 --- a/manila/share/drivers/cephfs/driver.py +++ b/manila/share/drivers/cephfs/driver.py @@ -27,6 +27,8 @@ import six from manila.common import constants from manila import exception from manila.i18n import _ +from manila.message import api as message_api +from manila.message import message_field from manila.share import driver from manila.share.drivers import ganesha from manila.share.drivers.ganesha import utils as ganesha_utils @@ -371,6 +373,7 @@ class NativeProtocolHelper(ganesha.NASHelperBase): def __init__(self, execute, config, **kwargs): self.volume_client = kwargs.pop('ceph_vol_client') + self.message_api = message_api.API() super(NativeProtocolHelper, self).__init__(execute, config, **kwargs) @@ -400,8 +403,7 @@ class NativeProtocolHelper(ganesha.NASHelperBase): def _allow_access(self, context, share, access, share_server=None): if access['access_type'] != CEPHX_ACCESS_TYPE: - raise exception.InvalidShareAccess( - reason=_("Only 'cephx' access type allowed.")) + raise exception.InvalidShareAccessType(type=access['access_type']) ceph_auth_id = access['access_to'] @@ -414,7 +416,7 @@ class NativeProtocolHelper(ganesha.NASHelperBase): error_message = (_('Ceph authentication ID %s must be different ' 'than the one the Manila service uses.') % ceph_auth_id) - raise exception.InvalidInput(message=error_message) + raise exception.InvalidShareAccess(reason=error_message) if not getattr(self.volume_client, 'version', None): if access['access_level'] == constants.ACCESS_LEVEL_RO: @@ -427,9 +429,18 @@ class NativeProtocolHelper(ganesha.NASHelperBase): cephfs_share_path(share), ceph_auth_id) else: readonly = access['access_level'] == constants.ACCESS_LEVEL_RO - auth_result = self.volume_client.authorize( - cephfs_share_path(share), ceph_auth_id, readonly=readonly, - tenant_id=share['project_id']) + try: + auth_result = self.volume_client.authorize( + cephfs_share_path(share), ceph_auth_id, readonly=readonly, + tenant_id=share['project_id']) + except Exception as e: + if 'not allowed' in str(e).lower(): + msg = ("Access to client %(client)s is not allowed. " + "Reason: %(reason)s") + msg_payload = {'client': ceph_auth_id, 'reason': e} + raise exception.InvalidShareAccess( + reason=msg % msg_payload) + raise return auth_result['auth_key'] @@ -448,7 +459,7 @@ class NativeProtocolHelper(ganesha.NASHelperBase): def update_access(self, context, share, access_rules, add_rules, delete_rules, share_server=None): - access_keys = {} + access_updates = {} if not (add_rules or delete_rules): # recovery/maintenance mode add_rules = access_rules @@ -480,13 +491,48 @@ class NativeProtocolHelper(ganesha.NASHelperBase): # access keys and ensure that after recovery, manila and the Ceph # backend are in sync. for rule in add_rules: - access_key = self._allow_access(context, share, rule) - access_keys.update({rule['access_id']: {'access_key': access_key}}) + try: + access_key = self._allow_access(context, share, rule) + except (exception.InvalidShareAccessLevel, + exception.InvalidShareAccessType): + self.message_api.create( + context, + message_field.Action.UPDATE_ACCESS_RULES, + share['project_id'], + resource_type=message_field.Resource.SHARE, + resource_id=share['share_id'], + detail=message_field.Detail.UNSUPPORTED_CLIENT_ACCESS) + log_args = {'id': rule['access_id'], + 'access_level': rule['access_level'], + 'access_to': rule['access_to']} + LOG.exception("Failed to provide %(access_level)s access to " + "%(access_to)s (Rule ID: %(id)s). Setting rule " + "to 'error' state.", log_args) + access_updates.update({rule['access_id']: {'state': 'error'}}) + except exception.InvalidShareAccess: + self.message_api.create( + context, + message_field.Action.UPDATE_ACCESS_RULES, + share['project_id'], + resource_type=message_field.Resource.SHARE, + resource_id=share['share_id'], + detail=message_field.Detail.FORBIDDEN_CLIENT_ACCESS) + log_args = {'id': rule['access_id'], + 'access_level': rule['access_level'], + 'access_to': rule['access_to']} + LOG.exception("Failed to provide %(access_level)s access to " + "%(access_to)s (Rule ID: %(id)s). Setting rule " + "to 'error' state.", log_args) + access_updates.update({rule['access_id']: {'state': 'error'}}) + else: + access_updates.update({ + rule['access_id']: {'access_key': access_key}, + }) for rule in delete_rules: self._deny_access(context, share, rule) - return access_keys + return access_updates def get_configured_ip_versions(self): return [4] diff --git a/manila/tests/share/drivers/cephfs/test_driver.py b/manila/tests/share/drivers/cephfs/test_driver.py index ab52a73191..52f15f939e 100644 --- a/manila/tests/share/drivers/cephfs/test_driver.py +++ b/manila/tests/share/drivers/cephfs/test_driver.py @@ -394,7 +394,7 @@ class NativeProtocolHelperTestCase(test.TestCase): super(NativeProtocolHelperTestCase, self).setUp() self.fake_conf = configuration.Configuration(None) self._context = context.get_admin_context() - self._share = fake_share.fake_share(share_proto='CEPHFS') + self._share = fake_share.fake_share_instance(share_proto='CEPHFS') self.fake_conf.set_default('driver_handles_share_servers', False) @@ -477,7 +477,7 @@ class NativeProtocolHelperTestCase(test.TestCase): tenant_id=self._share['project_id']) def test_allow_access_wrong_type(self): - self.assertRaises(exception.InvalidShareAccess, + self.assertRaises(exception.InvalidShareAccessType, self._native_protocol_helper._allow_access, self._context, self._share, { 'access_level': constants.ACCESS_LEVEL_RW, @@ -486,7 +486,7 @@ class NativeProtocolHelperTestCase(test.TestCase): }) def test_allow_access_same_cephx_id_as_manila_service(self): - self.assertRaises(exception.InvalidInput, + self.assertRaises(exception.InvalidShareAccess, self._native_protocol_helper._allow_access, self._context, self._share, { 'access_level': constants.ACCESS_LEVEL_RW, @@ -494,6 +494,23 @@ class NativeProtocolHelperTestCase(test.TestCase): 'access_to': 'manila', }) + def test_allow_access_to_preexisting_ceph_user(self): + + vc = self._native_protocol_helper.volume_client + msg = ("auth ID: admin exists and not created by " + "ceph_volume_client. Not allowed to modify") + self.mock_object(vc, 'authorize', + mock.Mock(side_effect=Exception(msg))) + + self.assertRaises(exception.InvalidShareAccess, + self._native_protocol_helper._allow_access, + self._context, self._share, + { + 'access_level': constants.ACCESS_LEVEL_RW, + 'access_type': 'cephx', + 'access_to': 'admin' + }) + def test_deny_access(self): vc = self._native_protocol_helper.volume_client self._native_protocol_helper._deny_access(self._context, self._share, { @@ -508,7 +525,6 @@ class NativeProtocolHelperTestCase(test.TestCase): "alice", volume_path=driver.cephfs_share_path(self._share)) def test_update_access_add_rm(self): - vc = self._native_protocol_helper.volume_client alice = { 'id': 'instance_mapping_id1', 'access_id': 'accessid1', @@ -519,22 +535,66 @@ class NativeProtocolHelperTestCase(test.TestCase): bob = { 'id': 'instance_mapping_id2', 'access_id': 'accessid2', - 'access_level': 'rw', + 'access_level': 'ro', 'access_type': 'cephx', 'access_to': 'bob' } + manila = { + 'id': 'instance_mapping_id3', + 'access_id': 'accessid3', + 'access_level': 'ro', + 'access_type': 'cephx', + 'access_to': 'manila' + } + admin = { + 'id': 'instance_mapping_id4', + 'access_id': 'accessid4', + 'access_level': 'rw', + 'access_type': 'cephx', + 'access_to': 'admin' + } + dabo = { + 'id': 'instance_mapping_id5', + 'access_id': 'accessid5', + 'access_level': 'rwx', + 'access_type': 'cephx', + 'access_to': 'dabo' + } + + allow_access_side_effects = [ + 'abc123', + exception.InvalidShareAccess(reason='not'), + exception.InvalidShareAccess(reason='allowed'), + exception.InvalidShareAccessLevel(level='rwx') + ] + self.mock_object(self._native_protocol_helper.message_api, 'create') + self.mock_object(self._native_protocol_helper, '_deny_access') + self.mock_object(self._native_protocol_helper, + '_allow_access', + mock.Mock(side_effect=allow_access_side_effects)) access_updates = self._native_protocol_helper.update_access( - self._context, self._share, access_rules=[alice], - add_rules=[alice], delete_rules=[bob]) + self._context, + self._share, + access_rules=[alice, manila, admin, dabo], + add_rules=[alice, manila, admin, dabo], + delete_rules=[bob]) + expected_access_updates = { + 'accessid1': {'access_key': 'abc123'}, + 'accessid3': {'state': 'error'}, + 'accessid4': {'state': 'error'}, + 'accessid5': {'state': 'error'} + } + self.assertEqual(expected_access_updates, access_updates) + self._native_protocol_helper._allow_access.assert_has_calls( + [mock.call(self._context, self._share, alice), + mock.call(self._context, self._share, manila), + mock.call(self._context, self._share, admin)]) + self._native_protocol_helper._deny_access.assert_called_once_with( + self._context, self._share, bob) self.assertEqual( - {'accessid1': {'access_key': 'abc123'}}, access_updates) - vc.authorize.assert_called_once_with( - driver.cephfs_share_path(self._share), "alice", readonly=False, - tenant_id=self._share['project_id']) - vc.deauthorize.assert_called_once_with( - driver.cephfs_share_path(self._share), "bob") + 3, self._native_protocol_helper.message_api.create.call_count) @ddt.data(None, 1) def test_update_access_all(self, volume_client_version): diff --git a/releasenotes/notes/bug-1904015-cve-2020-27781-cephx-asynchronous-msgs-6a683076a1fb5a54.yaml b/releasenotes/notes/bug-1904015-cve-2020-27781-cephx-asynchronous-msgs-6a683076a1fb5a54.yaml new file mode 100644 index 0000000000..917443b31c --- /dev/null +++ b/releasenotes/notes/bug-1904015-cve-2020-27781-cephx-asynchronous-msgs-6a683076a1fb5a54.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + New user messages now alert users of possible remediations during access + rule creation errors with CephFS shares. This includes hints to users to + not use cephx client users that are prohibited by CephFS or the share + driver. See `CVE-2020-27781 + `_ and + bug #1904015 `_ for more details.