Fix MITM vulnerability for Brocade FC SAN lookup
Refactor the Brocade FC SAN lookup service implementation to use common SSH utilities which already avoid MITM vulnerabilities. This is a follow-up to https://review.openstack.org/#/c/138526/ which reverted an incomplete fix for the MITM issues for Brocade. Change-Id: I2d87b55f56f08208f0da11cac40682d51da5b536 Closes-Bug: #1391311
This commit is contained in:
parent
bd9719508d
commit
46fdb37c68
|
@ -20,10 +20,11 @@
|
|||
"""Unit tests for brcd fc san lookup service."""
|
||||
|
||||
import mock
|
||||
from oslo_concurrency import processutils as putils
|
||||
from oslo_config import cfg
|
||||
import paramiko
|
||||
|
||||
from cinder import exception
|
||||
from cinder import ssh_utils
|
||||
from cinder import test
|
||||
from cinder.volume import configuration as conf
|
||||
import cinder.zonemanager.drivers.brocade.brcd_fc_san_lookup_service \
|
||||
|
@ -31,14 +32,41 @@ import cinder.zonemanager.drivers.brocade.brcd_fc_san_lookup_service \
|
|||
from cinder.zonemanager.drivers.brocade import fc_zone_constants
|
||||
|
||||
|
||||
nsshow = '20:1a:00:05:1e:e8:e3:29'
|
||||
switch_data = [' N 011a00;2,3;20:1a:00:05:1e:e8:e3:29;\
|
||||
20:1a:00:05:1e:e8:e3:29;na']
|
||||
nsshow_data = ['10:00:8c:7c:ff:52:3b:01', '20:24:00:02:ac:00:0a:50']
|
||||
parsed_switch_port_wwns = ['20:1a:00:05:1e:e8:e3:29',
|
||||
'10:00:00:90:fa:34:40:f6']
|
||||
switch_data = ("""
|
||||
Type Pid COS PortName NodeName TTL(sec)
|
||||
N 011a00; 2,3; %(port_1)s; 20:1a:00:05:1e:e8:e3:29; na
|
||||
FC4s: FCP
|
||||
PortSymb: [26] "222222 - 1:1:1 - LPe12442"
|
||||
NodeSymb: [32] "SomeSym 7211"
|
||||
Fabric Port Name: 20:1a:00:05:1e:e8:e3:29
|
||||
Permanent Port Name: 22:22:00:22:ac:00:bc:b0
|
||||
Port Index: 0
|
||||
Share Area: No
|
||||
Device Shared in Other AD: No
|
||||
Redirect: No
|
||||
Partial: No
|
||||
LSAN: No
|
||||
N 010100; 2,3; %(port_2)s; 20:00:00:00:af:00:00:af; na
|
||||
FC4s: FCP
|
||||
PortSymb: [26] "333333 - 1:1:1 - LPe12442"
|
||||
NodeSymb: [32] "SomeSym 2222"
|
||||
Fabric Port Name: 10:00:00:90:fa:34:40:f6
|
||||
Permanent Port Name: 22:22:00:22:ac:00:bc:b0
|
||||
Port Index: 0
|
||||
Share Area: No
|
||||
Device Shared in Other AD: No
|
||||
Redirect: No
|
||||
Partial: No
|
||||
LSAN: No""" % {'port_1': parsed_switch_port_wwns[0],
|
||||
'port_2': parsed_switch_port_wwns[1]})
|
||||
|
||||
_device_map_to_verify = {
|
||||
'BRCD_FAB_2': {
|
||||
'initiator_port_wwn_list': ['10008c7cff523b01'],
|
||||
'target_port_wwn_list': ['20240002ac000a50']}}
|
||||
'initiator_port_wwn_list': [parsed_switch_port_wwns[1].replace(':',
|
||||
'')],
|
||||
'target_port_wwn_list': [parsed_switch_port_wwns[0].replace(':', '')]}}
|
||||
|
||||
|
||||
class TestBrcdFCSanLookupService(brcd_lookup.BrcdFCSanLookupService,
|
||||
|
@ -46,7 +74,6 @@ class TestBrcdFCSanLookupService(brcd_lookup.BrcdFCSanLookupService,
|
|||
|
||||
def setUp(self):
|
||||
super(TestBrcdFCSanLookupService, self).setUp()
|
||||
self.client = paramiko.SSHClient()
|
||||
self.configuration = conf.Configuration(None)
|
||||
self.configuration.set_default('fc_fabric_names', 'BRCD_FAB_2',
|
||||
'fc-zone-manager')
|
||||
|
@ -73,57 +100,47 @@ class TestBrcdFCSanLookupService(brcd_lookup.BrcdFCSanLookupService,
|
|||
config = conf.Configuration(fc_fabric_opts, 'BRCD_FAB_2')
|
||||
self.fabric_configs = {'BRCD_FAB_2': config}
|
||||
|
||||
@mock.patch.object(paramiko.hostkeys.HostKeys, 'load')
|
||||
def test_create_ssh_client(self, load_mock):
|
||||
mock_args = {}
|
||||
mock_args['known_hosts_file'] = 'dummy_host_key_file'
|
||||
mock_args['missing_key_policy'] = paramiko.RejectPolicy()
|
||||
ssh_client = self.create_ssh_client(**mock_args)
|
||||
self.assertEqual('dummy_host_key_file', ssh_client._host_keys_filename)
|
||||
self.assertTrue(isinstance(ssh_client._policy, paramiko.RejectPolicy))
|
||||
mock_args = {}
|
||||
ssh_client = self.create_ssh_client(**mock_args)
|
||||
self.assertIsNone(ssh_client._host_keys_filename)
|
||||
self.assertTrue(isinstance(ssh_client._policy, paramiko.WarningPolicy))
|
||||
|
||||
@mock.patch.object(brcd_lookup.BrcdFCSanLookupService,
|
||||
'get_nameserver_info')
|
||||
def test_get_device_mapping_from_network(self, get_nameserver_info_mock):
|
||||
initiator_list = ['10008c7cff523b01']
|
||||
target_list = ['20240002ac000a50', '20240002ac000a40']
|
||||
with mock.patch.object(self.client, 'connect'):
|
||||
get_nameserver_info_mock.return_value = (nsshow_data)
|
||||
device_map = self.get_device_mapping_from_network(
|
||||
initiator_list, target_list)
|
||||
self.assertDictMatch(device_map, _device_map_to_verify)
|
||||
@mock.patch('cinder.zonemanager.drivers.brocade.brcd_fc_san_lookup_service'
|
||||
'.ssh_utils.SSHPool')
|
||||
def test_get_device_mapping_from_network(self, mock_ssh_pool,
|
||||
get_nameserver_info_mock):
|
||||
initiator_list = [parsed_switch_port_wwns[1]]
|
||||
target_list = [parsed_switch_port_wwns[0], '20240002ac000a40']
|
||||
get_nameserver_info_mock.return_value = parsed_switch_port_wwns
|
||||
device_map = self.get_device_mapping_from_network(
|
||||
initiator_list, target_list)
|
||||
self.assertDictMatch(device_map, _device_map_to_verify)
|
||||
|
||||
@mock.patch.object(brcd_lookup.BrcdFCSanLookupService, '_get_switch_data')
|
||||
def test_get_nameserver_info(self, get_switch_data_mock):
|
||||
ns_info_list = []
|
||||
ns_info_list_expected = ['20:1a:00:05:1e:e8:e3:29',
|
||||
'20:1a:00:05:1e:e8:e3:29']
|
||||
|
||||
get_switch_data_mock.return_value = (switch_data)
|
||||
ns_info_list = self.get_nameserver_info()
|
||||
# get_switch_data will be called twice with the results appended
|
||||
ns_info_list_expected = (parsed_switch_port_wwns +
|
||||
parsed_switch_port_wwns)
|
||||
|
||||
ns_info_list = self.get_nameserver_info(None)
|
||||
self.assertEqual(ns_info_list_expected, ns_info_list)
|
||||
|
||||
def test__get_switch_data(self):
|
||||
cmd = fc_zone_constants.NS_SHOW
|
||||
|
||||
with mock.patch.object(self.client, 'exec_command') \
|
||||
as exec_command_mock:
|
||||
exec_command_mock.return_value = (Stream(),
|
||||
Stream(nsshow),
|
||||
Stream())
|
||||
switch_data = self._get_switch_data(cmd)
|
||||
self.assertEqual(nsshow, switch_data)
|
||||
exec_command_mock.assert_called_once_with(cmd)
|
||||
@mock.patch.object(putils, 'ssh_execute', return_value=(switch_data, ''))
|
||||
@mock.patch.object(ssh_utils.SSHPool, 'item')
|
||||
def test__get_switch_data(self, ssh_pool_mock, ssh_execute_mock):
|
||||
actual_switch_data = self._get_switch_data(ssh_pool_mock,
|
||||
fc_zone_constants.NS_SHOW)
|
||||
self.assertEqual(actual_switch_data, switch_data)
|
||||
ssh_execute_mock.side_effect = putils.ProcessExecutionError()
|
||||
self.assertRaises(exception.FCSanLookupServiceException,
|
||||
self._get_switch_data, ssh_pool_mock,
|
||||
fc_zone_constants.NS_SHOW)
|
||||
|
||||
def test__parse_ns_output(self):
|
||||
invalid_switch_data = [' N 011a00;20:1a:00:05:1e:e8:e3:29']
|
||||
invalid_switch_data = ' N 011a00;20:1a:00:05:1e:e8:e3:29'
|
||||
return_wwn_list = []
|
||||
expected_wwn_list = ['20:1a:00:05:1e:e8:e3:29']
|
||||
return_wwn_list = self._parse_ns_output(switch_data)
|
||||
self.assertEqual(expected_wwn_list, return_wwn_list)
|
||||
self.assertEqual(parsed_switch_port_wwns, return_wwn_list)
|
||||
self.assertRaises(exception.InvalidParameterValue,
|
||||
self._parse_ns_output, invalid_switch_data)
|
||||
|
||||
|
@ -133,23 +150,3 @@ class TestBrcdFCSanLookupService(brcd_lookup.BrcdFCSanLookupService,
|
|||
expected_wwn_list = ['10:00:8c:7c:ff:52:3b:01']
|
||||
return_wwn_list.append(self.get_formatted_wwn(wwn_list[0]))
|
||||
self.assertEqual(expected_wwn_list, return_wwn_list)
|
||||
|
||||
|
||||
class Channel(object):
|
||||
def recv_exit_status(self):
|
||||
return 0
|
||||
|
||||
|
||||
class Stream(object):
|
||||
def __init__(self, buffer=''):
|
||||
self.buffer = buffer
|
||||
self.channel = Channel()
|
||||
|
||||
def readlines(self):
|
||||
return self.buffer
|
||||
|
||||
def close(self):
|
||||
pass
|
||||
|
||||
def flush(self):
|
||||
self.buffer = ''
|
||||
|
|
|
@ -16,13 +16,14 @@
|
|||
# under the License.
|
||||
#
|
||||
|
||||
from oslo_concurrency import processutils
|
||||
from oslo_log import log as logging
|
||||
from oslo_utils import excutils
|
||||
import paramiko
|
||||
import six
|
||||
|
||||
from cinder import exception
|
||||
from cinder.i18n import _, _LE
|
||||
from cinder import ssh_utils
|
||||
from cinder import utils
|
||||
from cinder.zonemanager.drivers.brocade import brcd_fabric_opts as fabric_opts
|
||||
import cinder.zonemanager.drivers.brocade.fc_zone_constants as zone_constant
|
||||
|
@ -46,7 +47,6 @@ class BrcdFCSanLookupService(fc_service.FCSanLookupService):
|
|||
super(BrcdFCSanLookupService, self).__init__(**kwargs)
|
||||
self.configuration = kwargs.get('configuration', None)
|
||||
self.create_configuration()
|
||||
self.client = self.create_ssh_client(**kwargs)
|
||||
|
||||
def create_configuration(self):
|
||||
"""Configuration specific to SAN context values."""
|
||||
|
@ -61,19 +61,6 @@ class BrcdFCSanLookupService(fc_service.FCSanLookupService):
|
|||
self.fabric_configs = fabric_opts.load_fabric_configurations(
|
||||
fabric_names)
|
||||
|
||||
def create_ssh_client(self, **kwargs):
|
||||
ssh_client = paramiko.SSHClient()
|
||||
known_hosts_file = kwargs.get('known_hosts_file', None)
|
||||
if known_hosts_file is None:
|
||||
ssh_client.load_system_host_keys()
|
||||
else:
|
||||
ssh_client.load_host_keys(known_hosts_file)
|
||||
missing_key_policy = kwargs.get('missing_key_policy', None)
|
||||
if missing_key_policy is None:
|
||||
missing_key_policy = paramiko.WarningPolicy()
|
||||
ssh_client.set_missing_host_key_policy(missing_key_policy)
|
||||
return ssh_client
|
||||
|
||||
def get_device_mapping_from_network(self,
|
||||
initiator_wwn_list,
|
||||
target_wwn_list):
|
||||
|
@ -126,15 +113,16 @@ class BrcdFCSanLookupService(fc_service.FCSanLookupService):
|
|||
fabric_port = self.fabric_configs[fabric_name].safe_get(
|
||||
'fc_fabric_port')
|
||||
|
||||
ssh_pool = ssh_utils.SSHPool(fabric_ip, fabric_port, None,
|
||||
fabric_user, password=fabric_pwd)
|
||||
|
||||
# Get name server data from fabric and find the targets
|
||||
# logged in
|
||||
nsinfo = ''
|
||||
try:
|
||||
LOG.debug("Getting name server data for "
|
||||
"fabric %s", fabric_ip)
|
||||
self.client.connect(
|
||||
fabric_ip, fabric_port, fabric_user, fabric_pwd)
|
||||
nsinfo = self.get_nameserver_info()
|
||||
nsinfo = self.get_nameserver_info(ssh_pool)
|
||||
except exception.FCSanLookupServiceException:
|
||||
with excutils.save_and_reraise_exception():
|
||||
LOG.error(_LE("Failed collecting name server info from"
|
||||
|
@ -145,8 +133,7 @@ class BrcdFCSanLookupService(fc_service.FCSanLookupService):
|
|||
) % {'fabric': fabric_ip, 'err': e}
|
||||
LOG.error(msg)
|
||||
raise exception.FCSanLookupServiceException(message=msg)
|
||||
finally:
|
||||
self.client.close()
|
||||
|
||||
LOG.debug("Lookup service:nsinfo-%s", nsinfo)
|
||||
LOG.debug("Lookup service:initiator list from "
|
||||
"caller-%s", formatted_initiator_list)
|
||||
|
@ -184,23 +171,28 @@ class BrcdFCSanLookupService(fc_service.FCSanLookupService):
|
|||
LOG.debug("Device map for SAN context: %s", device_map)
|
||||
return device_map
|
||||
|
||||
def get_nameserver_info(self):
|
||||
def get_nameserver_info(self, ssh_pool):
|
||||
"""Get name server data from fabric.
|
||||
|
||||
This method will return the connected node port wwn list(local
|
||||
and remote) for the given switch fabric
|
||||
|
||||
:param ssh_pool: SSH connections for the current fabric
|
||||
"""
|
||||
cli_output = None
|
||||
nsinfo_list = []
|
||||
try:
|
||||
cli_output = self._get_switch_data(zone_constant.NS_SHOW)
|
||||
cli_output = self._get_switch_data(ssh_pool,
|
||||
zone_constant.NS_SHOW)
|
||||
except exception.FCSanLookupServiceException:
|
||||
with excutils.save_and_reraise_exception():
|
||||
LOG.error(_LE("Failed collecting nsshow info for fabric"))
|
||||
if cli_output:
|
||||
nsinfo_list = self._parse_ns_output(cli_output)
|
||||
try:
|
||||
cli_output = self._get_switch_data(zone_constant.NS_CAM_SHOW)
|
||||
cli_output = self._get_switch_data(ssh_pool,
|
||||
zone_constant.NS_CAM_SHOW)
|
||||
|
||||
except exception.FCSanLookupServiceException:
|
||||
with excutils.save_and_reraise_exception():
|
||||
LOG.error(_LE("Failed collecting nscamshow"))
|
||||
|
@ -209,26 +201,19 @@ class BrcdFCSanLookupService(fc_service.FCSanLookupService):
|
|||
LOG.debug("Connector returning nsinfo-%s", nsinfo_list)
|
||||
return nsinfo_list
|
||||
|
||||
def _get_switch_data(self, cmd):
|
||||
stdin, stdout, stderr = None, None, None
|
||||
def _get_switch_data(self, ssh_pool, cmd):
|
||||
utils.check_ssh_injection([cmd])
|
||||
try:
|
||||
stdin, stdout, stderr = self.client.exec_command(cmd)
|
||||
switch_data = stdout.readlines()
|
||||
except paramiko.SSHException as e:
|
||||
msg = (_("SSH Command failed with error '%(err)s' "
|
||||
"'%(command)s'") % {'err': six.text_type(e),
|
||||
'command': cmd})
|
||||
LOG.error(msg)
|
||||
raise exception.FCSanLookupServiceException(message=msg)
|
||||
finally:
|
||||
if (stdin):
|
||||
stdin.flush()
|
||||
stdin.close()
|
||||
if (stdout):
|
||||
stdout.close()
|
||||
if (stderr):
|
||||
stderr.close()
|
||||
|
||||
with ssh_pool.item() as ssh:
|
||||
try:
|
||||
switch_data, err = processutils.ssh_execute(ssh, cmd)
|
||||
except processutils.ProcessExecutionError as e:
|
||||
msg = (_("SSH Command failed with error: '%(err)s', Command: "
|
||||
"'%(command)s'") % {'err': six.text_type(e),
|
||||
'command': cmd})
|
||||
LOG.error(msg)
|
||||
raise exception.FCSanLookupServiceException(message=msg)
|
||||
|
||||
return switch_data
|
||||
|
||||
def _parse_ns_output(self, switch_data):
|
||||
|
@ -239,12 +224,13 @@ class BrcdFCSanLookupService(fc_service.FCSanLookupService):
|
|||
:returns list of device port wwn from ns info
|
||||
"""
|
||||
nsinfo_list = []
|
||||
for line in switch_data:
|
||||
lines = switch_data.split('\n')
|
||||
for line in lines:
|
||||
if not(" NL " in line or " N " in line):
|
||||
continue
|
||||
linesplit = line.split(';')
|
||||
if len(linesplit) > 2:
|
||||
node_port_wwn = linesplit[2]
|
||||
node_port_wwn = linesplit[2].strip()
|
||||
nsinfo_list.append(node_port_wwn)
|
||||
else:
|
||||
msg = _("Malformed nameserver string: %s") % line
|
||||
|
|
Loading…
Reference in New Issue