[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 <gouthampravi@gmail.com>

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.")
 

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 = {

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]

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):

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
+    <https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-27781>`_ and
+    bug #1904015 <https://launchpad.net/bugs/1904015>`_ for more details.