From ed3f36bc94668abfa1a0b07cb8507e091f4bbcea Mon Sep 17 00:00:00 2001 From: yogeshprasad Date: Fri, 8 May 2015 16:07:22 +0530 Subject: [PATCH] Add chap support to CloudByte cinder driver CloudByte iSCSI cinder driver currently does not support iSCSI CHAP security setting. This opens a security vulnerability whereby any host machine can mount a CloudByte iSCSI volumes created via OpenStack. This patch adds CHAP authentication support for the CloudByte iSCSI cinder driver. Admin has to manually add the auth group and chap username/password in CloudByte storage. Driver uses the first user found in auth group. DocImpact Change-Id: I2c4c8320c7471e5cf2e2e242dbe1729c8974bf28 Implements: blueprint add-chap-authentication-support-to-cloudbyte-cinder-driver --- cinder/tests/unit/test_cloudbyte.py | 210 +++++++++++++++++-- cinder/volume/drivers/cloudbyte/cloudbyte.py | 163 ++++++++++++-- cinder/volume/drivers/cloudbyte/options.py | 9 +- 3 files changed, 340 insertions(+), 42 deletions(-) diff --git a/cinder/tests/unit/test_cloudbyte.py b/cinder/tests/unit/test_cloudbyte.py index a1ba649ef..0cbe523da 100644 --- a/cinder/tests/unit/test_cloudbyte.py +++ b/cinder/tests/unit/test_cloudbyte.py @@ -577,6 +577,34 @@ FAKE_UPDATE_QOS_GROUP_RESPONSE = """{ "updateqosresponse" : { }] }}""" +# A fake list iSCSI auth user response of cloudbyte's elasticenter +FAKE_LIST_ISCSI_AUTH_USER_RESPONSE = """{ "listiSCSIAuthUsersResponse" : { + "count":1 , + "authuser" : [{ + "id": "53d00164-a974-31b8-a854-bd346a8ea937", + "accountid": "12d41531-c41a-4ab7-abe2-ce0db2570119", + "authgroupid": "537744eb-c594-3145-85c0-96079922b894", + "chapusername": "fakeauthgroupchapuser", + "chappassword": "fakeauthgroupchapsecret", + "mutualchapusername": "fakeauthgroupmutualchapuser", + "mutualchappassword": "fakeauthgroupmutualchapsecret" + }] + }}""" + +# A fake list iSCSI auth group response of cloudbyte's elasticenter +FAKE_LIST_ISCSI_AUTH_GROUP_RESPONSE = """{ "listiSCSIAuthGroupResponse" : { + "count":2 , + "authgroup" : [{ + "id": "32d935ee-a60f-3681-b792-d8ccfe7e8e7f", + "name": "None", + "comment": "None" + }, { + "id": "537744eb-c594-3145-85c0-96079922b894", + "name": "fakeauthgroup", + "comment": "Fake Auth Group For Openstack " + }] + }}""" + # This dict maps the http commands of elasticenter # with its respective fake responses @@ -588,8 +616,6 @@ MAP_COMMAND_TO_FAKE_RESPONSE["listFileSystem"] = ( json.loads(FAKE_LIST_FILE_SYSTEM_RESPONSE)) MAP_COMMAND_TO_FAKE_RESPONSE["deleteSnapshot"] = ( json.loads(FAKE_DELETE_STORAGE_SNAPSHOT_RESPONSE)) -MAP_COMMAND_TO_FAKE_RESPONSE["listStorageSnapshots"] = ( - json.loads(FAKE_LIST_STORAGE_SNAPSHOTS_RESPONSE)) MAP_COMMAND_TO_FAKE_RESPONSE["updateVolumeiSCSIService"] = ( json.loads(FAKE_UPDATE_VOLUME_ISCSI_SERVICE_RESPONSE)) MAP_COMMAND_TO_FAKE_RESPONSE["createStorageSnapshot"] = ( @@ -616,12 +642,10 @@ MAP_COMMAND_TO_FAKE_RESPONSE['updateQosGroup'] = ( json.loads(FAKE_UPDATE_QOS_GROUP_RESPONSE)) MAP_COMMAND_TO_FAKE_RESPONSE['listStorageSnapshots'] = ( json.loads(FAKE_LIST_STORAGE_SNAPSHOTS_RESPONSE)) - -# This dict maps the http commands of elasticenter -# with its respective fake json responses -MAP_COMMAND_TO_FAKE_JSON_RESPONSE = {} - -MAP_COMMAND_TO_FAKE_JSON_RESPONSE["listTsm"] = FAKE_LIST_TSM_RESPONSE +MAP_COMMAND_TO_FAKE_RESPONSE['listiSCSIAuthUser'] = ( + json.loads(FAKE_LIST_ISCSI_AUTH_USER_RESPONSE)) +MAP_COMMAND_TO_FAKE_RESPONSE['listiSCSIAuthGroup'] = ( + json.loads(FAKE_LIST_ISCSI_AUTH_GROUP_RESPONSE)) class CloudByteISCSIDriverTestCase(testtools.TestCase): @@ -641,6 +665,7 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): # override some parts of driver configuration self.driver.configuration.cb_tsm_name = 'openstack' self.driver.configuration.cb_account_name = 'CustomerA' + self.driver.configuration.cb_auth_group = 'fakeauthgroup' def _side_effect_api_req(self, cmd, params, version='1.0'): """This is a side effect function. @@ -681,6 +706,30 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): return MAP_COMMAND_TO_FAKE_RESPONSE[cmd] + def _side_effect_api_req_to_list_iscsi_auth_group(self, cmd, params, + version='1.0'): + """This is a side effect function.""" + if cmd == 'listiSCSIAuthGroup': + return {} + + return MAP_COMMAND_TO_FAKE_RESPONSE[cmd] + + def _side_effect_api_req_to_list_iscsi_auth_user(self, cmd, params, + version='1.0'): + """This is a side effect function.""" + if cmd == 'listiSCSIAuthUser': + return {} + + return MAP_COMMAND_TO_FAKE_RESPONSE[cmd] + + def _side_effect_enable_chap(self): + """This is a side effect function.""" + self.driver.cb_use_chap = True + + def _side_effect_disable_chap(self): + """This is a side effect function.""" + self.driver.cb_use_chap = False + def _side_effect_api_req_to_list_filesystem( self, cmd, params, version='1.0'): """This is a side effect function.""" @@ -934,6 +983,9 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): # Test - I + # enable CHAP + self._side_effect_enable_chap() + # configure the mocks with respective side-effects mock_api_req.side_effect = self._side_effect_api_req @@ -945,15 +997,53 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): 'openstack', self.driver.configuration.cb_tsm_name) self.assertEqual( 'CustomerA', self.driver.configuration.cb_account_name) + self.assertEqual( + 'fakeauthgroup', self.driver.configuration.cb_auth_group) + + # assert the result + self.assertEqual( + 'CHAP fakeauthgroupchapuser fakeauthgroupchapsecret', + provider_details['provider_auth']) self.assertThat( provider_details['provider_location'], matchers.Contains('172.16.50.35:3260')) - # assert that 9 api calls were invoked - self.assertEqual(9, mock_api_req.call_count) + # assert the invoked api calls to CloudByte Storage + self.assertEqual(11, mock_api_req.call_count) # Test - II + # reset the mock + mock_api_req.reset_mock() + + # disable CHAP + self._side_effect_disable_chap() + + # configure the mocks with respective side-effects + mock_api_req.side_effect = self._side_effect_api_req + + # now run the test + provider_details = self.driver.create_volume(volume) + + # assert equality checks for certain configuration attributes + self.assertEqual( + 'openstack', self.driver.configuration.cb_tsm_name) + self.assertEqual( + 'CustomerA', self.driver.configuration.cb_account_name) + + # assert the result + self.assertEqual( + None, + provider_details['provider_auth']) + self.assertThat( + provider_details['provider_location'], + matchers.Contains('172.16.50.35:3260')) + + # assert the invoked api calls to CloudByte Storage + self.assertEqual(9, mock_api_req.call_count) + + # Test - III + # reconfigure the dependencies volume['id'] = 'NotExists' del volume['size'] @@ -970,7 +1060,7 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): "CloudByte storage."): self.driver.create_volume(volume) - # Test - III + # Test - IV # reconfigure the dependencies volume['id'] = 'abc' @@ -989,7 +1079,7 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): 'creating volume'): self.driver.create_volume(volume) - # Test - IV + # Test - V # reconfigure the dependencies # reset the mocks @@ -1122,11 +1212,51 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): # Test - I + # enable CHAP + self._side_effect_enable_chap() + + # configure the mocks with respective side-effects + mock_api_req.side_effect = self._side_effect_api_req + + # now run the test + provider_details = ( + self.driver.create_volume_from_snapshot(cloned_volume, snapshot)) + + # assert the result + self.assertEqual( + 'CHAP fakeauthgroupchapuser fakeauthgroupchapsecret', + provider_details['provider_auth']) + self.assertEqual( + '20.10.22.56:3260 ' + 'iqn.2014-06.acc1.openstacktsm:acc1DS1Snap1clone1 0', + provider_details['provider_location']) + + # assert the invoked api calls to CloudByte Storage + self.assertEqual(4, mock_api_req.call_count) + + # Test - II + + # reset the mocks + mock_api_req.reset_mock() + + # disable CHAP + self._side_effect_disable_chap() + # configure the mocks with respective side-effects mock_api_req.side_effect = self._side_effect_api_req # now run the test - self.driver.create_volume_from_snapshot(cloned_volume, snapshot) + provider_details = ( + self.driver.create_volume_from_snapshot(cloned_volume, snapshot)) + + # assert the result + self.assertEqual( + None, + provider_details['provider_auth']) + self.assertEqual( + '20.10.22.56:3260 ' + 'iqn.2014-06.acc1.openstacktsm:acc1DS1Snap1clone1 0', + provider_details['provider_location']) # assert n api calls were invoked self.assertEqual(1, mock_api_req.call_count) @@ -1160,7 +1290,10 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): '_api_request_for_cloudbyte') def test_create_export(self, mock_api_req): - # prepare the input test data + # Test - I + + # enable CHAP + self._side_effect_enable_chap() # configure the mocks with respective side-effects mock_api_req.side_effect = self._side_effect_api_req @@ -1169,13 +1302,35 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): model_update = self.driver.create_export({}, {}) # assert the result - self.assertEqual(None, model_update['provider_auth']) + self.assertEqual('CHAP fakeauthgroupchapuser fakeauthgroupchapsecret', + model_update['provider_auth']) + + # Test - II + + # reset the mocks + mock_api_req.reset_mock() + + # disable CHAP + self._side_effect_disable_chap() + + # configure the mocks with respective side-effects + mock_api_req.side_effect = self._side_effect_api_req + + # now run the test + model_update = self.driver.create_export({}, {}) + + # assert the result + self.assertEqual(None, + model_update['provider_auth']) @mock.patch.object(cloudbyte.CloudByteISCSIDriver, '_api_request_for_cloudbyte') def test_ensure_export(self, mock_api_req): - # prepare the input test data + # Test - I + + # enable CHAP + self._side_effect_enable_chap() # configure the mock with respective side-effects mock_api_req.side_effect = self._side_effect_api_req @@ -1184,14 +1339,31 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): model_update = self.driver.ensure_export({}, {}) # assert the result to have a provider_auth attribute - self.assertEqual(None, model_update['provider_auth']) + self.assertEqual('CHAP fakeauthgroupchapuser fakeauthgroupchapsecret', + model_update['provider_auth']) + + # Test - II + + # reset the mocks + mock_api_req.reset_mock() + + # disable CHAP + self._side_effect_disable_chap() + + # configure the mocks with respective side-effects + mock_api_req.side_effect = self._side_effect_api_req + + # now run the test + model_update = self.driver.create_export({}, {}) + + # assert the result + self.assertEqual(None, + model_update['provider_auth']) @mock.patch.object(cloudbyte.CloudByteISCSIDriver, '_api_request_for_cloudbyte') def test_get_volume_stats(self, mock_api_req): - # prepare the input test data - # configure the mock with a side-effect mock_api_req.side_effect = self._side_effect_api_req diff --git a/cinder/volume/drivers/cloudbyte/cloudbyte.py b/cinder/volume/drivers/cloudbyte/cloudbyte.py index 436fefc65..7bd5f79e0 100644 --- a/cinder/volume/drivers/cloudbyte/cloudbyte.py +++ b/cinder/volume/drivers/cloudbyte/cloudbyte.py @@ -36,9 +36,10 @@ class CloudByteISCSIDriver(san.SanISCSIDriver): Version history: 1.0.0 - Initial driver + 1.1.0 - Add chap support and minor bug fixes """ - VERSION = '1.0.0' + VERSION = '1.1.0' volume_stats = {} def __init__(self, *args, **kwargs): @@ -49,6 +50,7 @@ class CloudByteISCSIDriver(san.SanISCSIDriver): options.cloudbyte_create_volume_opts) self.configuration.append_config_values( options.cloudbyte_connection_opts) + self.cb_use_chap = self.configuration.use_chap_auth self.get_volume_stats() def _get_url(self, cmd, params, apikey): @@ -374,7 +376,7 @@ class CloudByteISCSIDriver(san.SanISCSIDriver): return qosgroup_id - def _build_provider_details_from_volume(self, volume): + def _build_provider_details_from_volume(self, volume, chap): model_update = {} model_update['provider_location'] = ( @@ -384,14 +386,21 @@ class CloudByteISCSIDriver(san.SanISCSIDriver): # Will provide CHAP Authentication on forthcoming patches/release model_update['provider_auth'] = None + if chap: + model_update['provider_auth'] = ('CHAP %(username)s %(password)s' + % chap) + model_update['provider_id'] = volume['id'] - LOG.debug("CloudByte volume [%(vol)s] properties: [%(props)s].", - {'vol': volume['iqnname'], 'props': model_update}) + LOG.debug("CloudByte volume iqn: [%(iqn)s] provider id: [%(proid)s].", + {'iqn': volume['iqnname'], 'proid': volume['id']}) return model_update - def _build_provider_details_from_response(self, cb_volumes, volume_name): + def _build_provider_details_from_response(self, + cb_volumes, + volume_name, + chap): """Get provider information.""" model_update = {} @@ -399,7 +408,8 @@ class CloudByteISCSIDriver(san.SanISCSIDriver): for vol in volumes: if vol['name'] == volume_name: - model_update = self._build_provider_details_from_volume(vol) + model_update = self._build_provider_details_from_volume(vol, + chap) break return model_update @@ -457,12 +467,16 @@ class CloudByteISCSIDriver(san.SanISCSIDriver): else: return iscsi_id - def _request_update_iscsi_service(self, iscsi_id, ig_id): + def _request_update_iscsi_service(self, iscsi_id, ig_id, ag_id): params = { "id": iscsi_id, "igid": ig_id } + if ag_id: + params['authgroupid'] = ag_id + params['authmethod'] = "CHAP" + self._api_request_for_cloudbyte( 'updateVolumeiSCSIService', params) @@ -570,6 +584,97 @@ class CloudByteISCSIDriver(san.SanISCSIDriver): return data + def _get_auth_group_id_from_response(self, data): + """Find iSCSI auth group id.""" + + chap_group = self.configuration.cb_auth_group + + ag_list_res = data.get('listiSCSIAuthGroupResponse') + + if ag_list_res is None: + msg = _("Null response received from CloudByte's " + "list iscsi auth groups.") + raise exception.VolumeBackendAPIException(data=msg) + + ag_list = ag_list_res.get('authgroup') + + if ag_list is None: + msg = _('No iscsi auth groups were found in CloudByte.') + raise exception.VolumeBackendAPIException(data=msg) + + ag_id = None + + for ag in ag_list: + if ag.get('name') == chap_group: + ag_id = ag['id'] + break + else: + msg = _("Auth group [%s] details not found in " + "CloudByte storage.") % chap_group + raise exception.VolumeBackendAPIException(data=msg) + + return ag_id + + def _get_auth_group_info(self, account_id, ag_id): + """Fetch the auth group details.""" + + params = {"accountid": account_id, "authgroupid": ag_id} + + auth_users = self._api_request_for_cloudbyte( + 'listiSCSIAuthUser', params) + + auth_user_details_res = auth_users.get('listiSCSIAuthUsersResponse') + + if auth_user_details_res is None: + msg = _("No response was received from CloudByte storage " + "list iSCSI auth user API call.") + raise exception.VolumeBackendAPIException(data=msg) + + auth_user_details = auth_user_details_res.get('authuser') + + if auth_user_details is None: + msg = _("Auth user details not found in CloudByte storage.") + raise exception.VolumeBackendAPIException(data=msg) + + chapuser = auth_user_details[0].get('chapusername') + chappassword = auth_user_details[0].get('chappassword') + + if chapuser is None or chappassword is None: + msg = _("Invalid chap user details found in CloudByte storage.") + raise exception.VolumeBackendAPIException(data=msg) + + data = {'username': chapuser, 'password': chappassword, 'ag_id': ag_id} + + return data + + def _get_chap_info(self, account_id): + """Fetch the chap details.""" + + params = {"accountid": account_id} + + iscsi_auth_data = self._api_request_for_cloudbyte( + 'listiSCSIAuthGroup', params) + + ag_id = self._get_auth_group_id_from_response( + iscsi_auth_data) + + return self._get_auth_group_info(account_id, ag_id) + + def _export(self): + model_update = {'provider_auth': None} + + if self.cb_use_chap is True: + account_name = self.configuration.cb_account_name + + account_id = self._get_account_id_from_name(account_name) + + chap = self._get_chap_info(account_id) + + model_update['provider_auth'] = ('CHAP %(username)s %(password)s' + % chap) + + return model_update + def create_volume(self, volume): tsm_name = self.configuration.cb_tsm_name @@ -636,16 +741,25 @@ class CloudByteISCSIDriver(san.SanISCSIDriver): LOG.debug("Updating iscsi service for CloudByte volume [%s].", cb_volume_name) + ag_id = None + chap_info = {} + + if self.cb_use_chap is True: + chap_info = self._get_chap_info(account_id) + ag_id = chap_info['ag_id'] + # Update the iscsi service with above fetched iscsi_id & ig_id - self._request_update_iscsi_service(iscsi_id, ig_id) + self._request_update_iscsi_service(iscsi_id, ig_id, ag_id) LOG.debug("CloudByte volume [%(vol)s] updated with " - "iscsi id [%(iscsi)s] and ig id [%(ig)s].", - {'vol': cb_volume_name, 'iscsi': iscsi_id, 'ig': ig_id}) + "iscsi id [%(iscsi)s] and initiator group [%(ig)s] and " + "authentication group [%(ag)s].", + {'vol': cb_volume_name, 'iscsi': iscsi_id, + 'ig': ig_id, 'ag': ag_id}) # Provide the model after successful completion of above steps provider = self._build_provider_details_from_response( - cb_volumes, cb_volume_name) + cb_volumes, cb_volume_name, chap_info) LOG.info(_LI("Successfully created a CloudByte volume [%(cb_vol)s] " "w.r.t OpenStack volume [%(stack_vol)s]."), @@ -829,7 +943,20 @@ class CloudByteISCSIDriver(san.SanISCSIDriver): 'cb_snap': cb_snapshot_path, 'stack_vol': parent_volume_id}) - return self._build_provider_details_from_volume(cb_vol) + chap_info = {} + + if self.cb_use_chap is True: + account_name = self.configuration.cb_account_name + + # Get account id of this account + account_id = self._get_account_id_from_name(account_name) + + chap_info = self._get_chap_info(account_id) + + model_update = self._build_provider_details_from_volume(cb_vol, + chap_info) + + return model_update def delete_snapshot(self, snapshot): """Delete a snapshot at CloudByte.""" @@ -888,21 +1015,13 @@ class CloudByteISCSIDriver(san.SanISCSIDriver): def create_export(self, context, volume): """Setup the iscsi export info.""" - model_update = {} - - # Will provide CHAP Authentication on forthcoming patches/release - model_update['provider_auth'] = None - return model_update + return self._export() def ensure_export(self, context, volume): """Verify the iscsi export info.""" - model_update = {} - # Will provide CHAP Authentication on forthcoming patches/release - model_update['provider_auth'] = None - - return model_update + return self._export() def get_volume_stats(self, refresh=False): """Get volume statistics. diff --git a/cinder/volume/drivers/cloudbyte/options.py b/cinder/volume/drivers/cloudbyte/options.py index 7ca3128ee..581e3963f 100644 --- a/cinder/volume/drivers/cloudbyte/options.py +++ b/cinder/volume/drivers/cloudbyte/options.py @@ -38,7 +38,14 @@ cloudbyte_connection_opts = [ default=3, help="Will confirm a successful volume " "creation in CloudByte storage by making " - "this many number of attempts."), ] + "this many number of attempts."), + cfg.StrOpt("cb_auth_group", + default="None", + help="This corresponds to the discovery authentication " + "group in CloudByte storage. " + "Chap users are added to this group. " + "Driver uses the first user found for this group. " + "Default value is None."), ] cloudbyte_add_qosgroup_opts = [ cfg.DictOpt('cb_add_qosgroup', -- 2.45.2