From d85ce6202eeec96ec0385a258b58cb2ce8fc0b2f Mon Sep 17 00:00:00 2001 From: ashrod98 Date: Fri, 30 Jun 2023 15:53:47 +0000 Subject: [PATCH] Conditional Import for FIPS Compliance Conditionally Import Parakimo Separate SSH functions into ssh_utils.py for safe conditional import. Change-Id: Ia1a3ee69bef76b52e4e6df1e73488c018ac0f3c9 (cherry picked from commit 47e7c78f3774d12d556e9ab51f3e04eb5432c24b) --- manila/exception.py | 4 + .../drivers/dell_emc/common/enas/connector.py | 12 +- manila/share/drivers/ganesha/utils.py | 3 +- manila/share/drivers/generic.py | 15 +- manila/share/drivers/hdfs/hdfs_native.py | 17 +- manila/share/drivers/hitachi/hnas/ssh.py | 13 +- manila/share/drivers/ibm/gpfs.py | 17 +- .../drivers/infortrend/infortrend_nas.py | 26 +-- .../drivers/inspur/instorage/cli_helper.py | 3 +- manila/share/drivers/maprfs/driver_util.py | 17 +- manila/ssh_utils.py | 132 ++++++++++++ .../dell_emc/common/enas/test_connector.py | 10 +- .../tests/share/drivers/ganesha/test_utils.py | 2 +- .../share/drivers/hdfs/test_hdfs_native.py | 13 +- .../share/drivers/hitachi/hnas/test_ssh.py | 4 +- manila/tests/share/drivers/ibm/test_gpfs.py | 9 +- .../inspur/instorage/test_instorage.py | 7 +- .../tests/share/drivers/maprfs/test_maprfs.py | 13 +- manila/tests/share/drivers/test_generic.py | 7 +- manila/tests/test_ssh_utils.py | 188 ++++++++++++++++++ manila/tests/test_utils.py | 167 ---------------- manila/utils.py | 98 --------- 22 files changed, 432 insertions(+), 345 deletions(-) create mode 100644 manila/ssh_utils.py create mode 100644 manila/tests/test_ssh_utils.py diff --git a/manila/exception.py b/manila/exception.py index 1fc31d17a1..5335ee7dcb 100644 --- a/manila/exception.py +++ b/manila/exception.py @@ -574,6 +574,10 @@ class ShareBackendException(ManilaException): message = _("Share backend error: %(msg)s.") +class RequirementMissing(ManilaException): + message = _("Requirement %(req)s is not installed.") + + class ExportLocationNotFound(NotFound): message = _("Export location %(uuid)s could not be found.") diff --git a/manila/share/drivers/dell_emc/common/enas/connector.py b/manila/share/drivers/dell_emc/common/enas/connector.py index 7872670d03..426e7de3cb 100644 --- a/manila/share/drivers/dell_emc/common/enas/connector.py +++ b/manila/share/drivers/dell_emc/common/enas/connector.py @@ -26,7 +26,7 @@ from manila import exception from manila.i18n import _ from manila.share.drivers.dell_emc.common.enas import constants from manila.share.drivers.dell_emc.common.enas import utils as enas_utils -from manila import utils +from manila import ssh_utils LOG = log.getLogger(__name__) @@ -140,11 +140,11 @@ class SSHConnector(object): self.password = configuration.emc_nas_password self.debug = debug - self.sshpool = utils.SSHPool(ip=self.storage_ip, - port=22, - conn_timeout=None, - login=self.username, - password=self.password) + self.sshpool = ssh_utils.SSHPool(ip=self.storage_ip, + port=22, + conn_timeout=None, + login=self.username, + password=self.password) def run_ssh(self, cmd_list, check_exit_code=False): command = ' '.join(pipes.quote(cmd_arg) for cmd_arg in cmd_list) diff --git a/manila/share/drivers/ganesha/utils.py b/manila/share/drivers/ganesha/utils.py index 08e366e3db..7224b86ecb 100644 --- a/manila/share/drivers/ganesha/utils.py +++ b/manila/share/drivers/ganesha/utils.py @@ -21,6 +21,7 @@ from oslo_log import log from manila import exception from manila.i18n import _ +from manila import ssh_utils from manila import utils LOG = log.getLogger(__name__) @@ -63,7 +64,7 @@ class SSHExecutor(object): """Callable encapsulating exec through ssh.""" def __init__(self, *args, **kwargs): - self.pool = utils.SSHPool(*args, **kwargs) + self.pool = ssh_utils.SSHPool(*args, **kwargs) def __call__(self, *args, **kwargs): # argument with identifier 'run_as_root=' is not accepted by diff --git a/manila/share/drivers/generic.py b/manila/share/drivers/generic.py index ff8ff83caf..d7f08a0586 100644 --- a/manila/share/drivers/generic.py +++ b/manila/share/drivers/generic.py @@ -31,6 +31,7 @@ from manila import exception from manila.i18n import _ from manila.share import driver from manila.share.drivers import service_instance +from manila import ssh_utils from manila import utils from manila import volume @@ -144,13 +145,13 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver): connection = self.ssh_connections.get(server['instance_id']) ssh_conn_timeout = self.configuration.ssh_conn_timeout if not connection: - ssh_pool = utils.SSHPool(server['ip'], - 22, - ssh_conn_timeout, - server['username'], - server.get('password'), - server.get('pk_path'), - max_size=1) + ssh_pool = ssh_utils.SSHPool(server['ip'], + 22, + ssh_conn_timeout, + server['username'], + server.get('password'), + server.get('pk_path'), + max_size=1) ssh = ssh_pool.create() self.ssh_connections[server['instance_id']] = (ssh_pool, ssh) else: diff --git a/manila/share/drivers/hdfs/hdfs_native.py b/manila/share/drivers/hdfs/hdfs_native.py index 982825af6f..e5993c84e4 100644 --- a/manila/share/drivers/hdfs/hdfs_native.py +++ b/manila/share/drivers/hdfs/hdfs_native.py @@ -38,6 +38,7 @@ from oslo_utils import units from manila import exception from manila.i18n import _ from manila.share import driver +from manila import ssh_utils from manila import utils LOG = log.getLogger(__name__) @@ -124,14 +125,14 @@ class HDFSNativeShareDriver(driver.ExecuteMixin, driver.ShareDriver): min_size = self.configuration.ssh_min_pool_conn max_size = self.configuration.ssh_max_pool_conn - ssh_pool = utils.SSHPool(host, - hdfs_ssh_port, - ssh_conn_timeout, - hdfs_ssh_name, - password=password, - privatekey=privatekey, - min_size=min_size, - max_size=max_size) + ssh_pool = ssh_utils.SSHPool(host, + hdfs_ssh_port, + ssh_conn_timeout, + hdfs_ssh_name, + password=password, + privatekey=privatekey, + min_size=min_size, + max_size=max_size) ssh = ssh_pool.create() self.ssh_connections[host] = (ssh_pool, ssh) else: diff --git a/manila/share/drivers/hitachi/hnas/ssh.py b/manila/share/drivers/hitachi/hnas/ssh.py index e624cfbb99..7093713b11 100644 --- a/manila/share/drivers/hitachi/hnas/ssh.py +++ b/manila/share/drivers/hitachi/hnas/ssh.py @@ -24,6 +24,7 @@ import time from manila import exception from manila.i18n import _ +from manila import ssh_utils from manila import utils as mutils LOG = log.getLogger(__name__) @@ -633,12 +634,12 @@ class HNASSSHBackend(object): commands = ' '.join(commands) if not self.sshpool: - self.sshpool = mutils.SSHPool(ip=self.ip, - port=self.port, - conn_timeout=None, - login=self.user, - password=self.password, - privatekey=self.priv_key) + self.sshpool = ssh_utils.SSHPool(ip=self.ip, + port=self.port, + conn_timeout=None, + login=self.user, + password=self.password, + privatekey=self.priv_key) with self.sshpool.item() as ssh: ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) try: diff --git a/manila/share/drivers/ibm/gpfs.py b/manila/share/drivers/ibm/gpfs.py index 53f20063b4..74905e5b0d 100644 --- a/manila/share/drivers/ibm/gpfs.py +++ b/manila/share/drivers/ibm/gpfs.py @@ -48,6 +48,7 @@ from manila.i18n import _ from manila.share import driver from manila.share.drivers.helpers import NFSHelper from manila.share import share_types +from manila import ssh_utils from manila import utils LOG = log.getLogger(__name__) @@ -175,14 +176,14 @@ class GPFSShareDriver(driver.ExecuteMixin, driver.GaneshaMixin, min_size = self.configuration.ssh_min_pool_conn max_size = self.configuration.ssh_max_pool_conn - self.sshpool = utils.SSHPool(host, - gpfs_ssh_port, - ssh_conn_timeout, - gpfs_ssh_login, - password=password, - privatekey=privatekey, - min_size=min_size, - max_size=max_size) + self.sshpool = ssh_utils.SSHPool(host, + gpfs_ssh_port, + ssh_conn_timeout, + gpfs_ssh_login, + password=password, + privatekey=privatekey, + min_size=min_size, + max_size=max_size) try: with self.sshpool.item() as ssh: return self._gpfs_ssh_execute( diff --git a/manila/share/drivers/infortrend/infortrend_nas.py b/manila/share/drivers/infortrend/infortrend_nas.py index 5b3ce169cf..c9cbfe08c3 100644 --- a/manila/share/drivers/infortrend/infortrend_nas.py +++ b/manila/share/drivers/infortrend/infortrend_nas.py @@ -24,8 +24,10 @@ from manila.common import constants from manila import exception from manila.i18n import _ from manila.share import utils as share_utils +from manila import ssh_utils from manila import utils as manila_utils + LOG = log.getLogger(__name__) @@ -120,21 +122,21 @@ class InfortrendNAS(object): def _init_connect(self): if not (self.sshpool and self.ssh): - self.sshpool = manila_utils.SSHPool(ip=self.nas_ip, - port=self.port, - conn_timeout=None, - login=self.username, - password=self.password, - privatekey=self.ssh_key) + self.sshpool = ssh_utils.SSHPool(ip=self.nas_ip, + port=self.port, + conn_timeout=None, + login=self.username, + password=self.password, + privatekey=self.ssh_key) self.ssh = self.sshpool.create() if not self.ssh.get_transport().is_active(): - self.sshpool = manila_utils.SSHPool(ip=self.nas_ip, - port=self.port, - conn_timeout=None, - login=self.username, - password=self.password, - privatekey=self.ssh_key) + self.sshpool = ssh_utils.SSHPool(ip=self.nas_ip, + port=self.port, + conn_timeout=None, + login=self.username, + password=self.password, + privatekey=self.ssh_key) self.ssh = self.sshpool.create() LOG.debug('NAScmd [%s@%s] start!', self.username, self.nas_ip) diff --git a/manila/share/drivers/inspur/instorage/cli_helper.py b/manila/share/drivers/inspur/instorage/cli_helper.py index 5359ae9a53..1302c5437c 100644 --- a/manila/share/drivers/inspur/instorage/cli_helper.py +++ b/manila/share/drivers/inspur/instorage/cli_helper.py @@ -28,6 +28,7 @@ from oslo_utils import excutils from manila import exception from manila.i18n import _ +from manila import ssh_utils from manila import utils as manila_utils LOG = log.getLogger(__name__) @@ -55,7 +56,7 @@ class SSHRunner(object): command = ' '.join(cmd_list) if not self.sshpool: try: - self.sshpool = manila_utils.SSHPool( + self.sshpool = ssh_utils.SSHPool( self.host, self.port, self.ssh_conn_timeout, diff --git a/manila/share/drivers/maprfs/driver_util.py b/manila/share/drivers/maprfs/driver_util.py index 715f9bcc56..0a64cd220e 100644 --- a/manila/share/drivers/maprfs/driver_util.py +++ b/manila/share/drivers/maprfs/driver_util.py @@ -26,6 +26,7 @@ from oslo_log import log from manila.common import constants from manila import exception from manila.i18n import _ +from manila import ssh_utils from manila import utils LOG = log.getLogger(__name__) @@ -92,14 +93,14 @@ class BaseDriverUtil(object): min_size = self.configuration.ssh_min_pool_conn max_size = self.configuration.ssh_max_pool_conn - ssh_pool = utils.SSHPool(host, - remote_ssh_port, - ssh_conn_timeout, - ssh_name, - password=password, - privatekey=private_key, - min_size=min_size, - max_size=max_size) + ssh_pool = ssh_utils.SSHPool(host, + remote_ssh_port, + ssh_conn_timeout, + ssh_name, + password=password, + privatekey=private_key, + min_size=min_size, + max_size=max_size) ssh = ssh_pool.create() self.ssh_connections[host] = (ssh_pool, ssh) else: diff --git a/manila/ssh_utils.py b/manila/ssh_utils.py new file mode 100644 index 0000000000..6a897d680b --- /dev/null +++ b/manila/ssh_utils.py @@ -0,0 +1,132 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Ssh utilities.""" + +import logging +import os + +from eventlet import pools +from oslo_config import cfg +from oslo_log import log +from oslo_utils.secretutils import md5 + +from manila import exception +from manila.i18n import _ + + +try: + import paramiko +except ImportError: + paramiko = None + +CONF = cfg.CONF +LOG = log.getLogger(__name__) +if getattr(CONF, 'debug', False): + logging.getLogger("paramiko").setLevel(logging.DEBUG) + + +def get_fingerprint(self): + """Patch paramiko + + This method needs to be patched to allow paramiko to work under FIPS. + Until the patch to do this merges, patch paramiko here. + + TODO(carloss) Remove this when paramiko is patched. + See https://github.com/paramiko/paramiko/pull/1928 + """ + return md5(self.asbytes(), usedforsecurity=False).digest() + + +if paramiko is None: + raise exception.RequirementMissing(req='paramiko') + +paramiko.pkey.PKey.get_fingerprint = get_fingerprint + + +class SSHPool(pools.Pool): + """A simple eventlet pool to hold ssh connections.""" + + def __init__(self, ip, port, conn_timeout, login, password=None, + privatekey=None, *args, **kwargs): + self.ip = ip + self.port = port + self.login = login + self.password = password + self.conn_timeout = conn_timeout if conn_timeout else None + self.path_to_private_key = privatekey + super(SSHPool, self).__init__(*args, **kwargs) + + def create(self): # pylint: disable=method-hidden + ssh = paramiko.SSHClient() + ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) + look_for_keys = True + if self.path_to_private_key: + self.path_to_private_key = os.path.expanduser( + self.path_to_private_key) + look_for_keys = False + elif self.password: + look_for_keys = False + try: + LOG.debug("ssh.connect: ip: %s, port: %s, look_for_keys: %s, " + "timeout: %s, banner_timeout: %s", + self.ip, + self.port, + look_for_keys, + self.conn_timeout, + self.conn_timeout) + ssh.connect(self.ip, + port=self.port, + username=self.login, + password=self.password, + key_filename=self.path_to_private_key, + look_for_keys=look_for_keys, + timeout=self.conn_timeout, + banner_timeout=self.conn_timeout) + if self.conn_timeout: + transport = ssh.get_transport() + transport.set_keepalive(self.conn_timeout) + return ssh + except Exception as e: + msg = _("Check whether private key or password are correctly " + "set. Error connecting via ssh: %s") % e + LOG.error(msg) + raise exception.SSHException(msg) + + def get(self): + """Return an item from the pool, when one is available. + + This may cause the calling greenthread to block. Check if a + connection is active before returning it. For dead connections + create and return a new connection. + """ + if self.free_items: + conn = self.free_items.popleft() + if conn: + if conn.get_transport().is_active(): + return conn + else: + conn.close() + return self.create() + if self.current_size < self.max_size: + created = self.create() + self.current_size += 1 + return created + return self.channel.get() + + def remove(self, ssh): + """Close an ssh client and remove it from free_items.""" + ssh.close() + if ssh in self.free_items: + self.free_items.remove(ssh) + if self.current_size > 0: + self.current_size -= 1 diff --git a/manila/tests/share/drivers/dell_emc/common/enas/test_connector.py b/manila/tests/share/drivers/dell_emc/common/enas/test_connector.py index 36c27eec95..3600001f46 100644 --- a/manila/tests/share/drivers/dell_emc/common/enas/test_connector.py +++ b/manila/tests/share/drivers/dell_emc/common/enas/test_connector.py @@ -23,10 +23,10 @@ from oslo_concurrency import processutils from manila import exception from manila.share import configuration as conf from manila.share.drivers.dell_emc.common.enas import connector +from manila import ssh_utils from manila import test from manila.tests.share.drivers.dell_emc.common.enas import fakes from manila.tests.share.drivers.dell_emc.common.enas import utils as enas_utils -from manila import utils class XMLAPIConnectorTestData(object): @@ -165,12 +165,12 @@ class CmdConnectorTest(test.TestCase): self.configuration.emc_ssl_cert_path = None self.sshpool = MockSSHPool() - with mock.patch.object(utils, "SSHPool", + with mock.patch.object(ssh_utils, "SSHPool", mock.Mock(return_value=self.sshpool)): self.CmdHelper = connector.SSHConnector( configuration=self.configuration, debug=False) - utils.SSHPool.assert_called_once_with( + ssh_utils.SSHPool.assert_called_once_with( ip=fakes.FakeData.emc_nas_server, port=22, conn_timeout=None, @@ -207,7 +207,7 @@ class CmdConnectorTest(test.TestCase): sshpool = MockSSHPool() - with mock.patch.object(utils, "SSHPool", + with mock.patch.object(ssh_utils, "SSHPool", mock.Mock(return_value=sshpool)): self.CmdHelper = connector.SSHConnector(self.configuration) @@ -216,7 +216,7 @@ class CmdConnectorTest(test.TestCase): cmd_list, True) - utils.SSHPool.assert_called_once_with( + ssh_utils.SSHPool.assert_called_once_with( ip=fakes.FakeData.emc_nas_server, port=22, conn_timeout=None, diff --git a/manila/tests/share/drivers/ganesha/test_utils.py b/manila/tests/share/drivers/ganesha/test_utils.py index 0c276adfb4..6b76436f8b 100644 --- a/manila/tests/share/drivers/ganesha/test_utils.py +++ b/manila/tests/share/drivers/ganesha/test_utils.py @@ -128,7 +128,7 @@ class SSHExecutorTestCase(test.TestCase): @ddt.unpack def test_call_ssh_exec_object_with_run_as_root( self, run_as_root, expected_prefix): - with mock.patch.object(ganesha_utils.utils, 'SSHPool'): + with mock.patch.object(ganesha_utils.ssh_utils, 'SSHPool'): self.execute = ganesha_utils.SSHExecutor() fake_ssh_object = mock.Mock() self.mock_object(self.execute.pool, 'get', diff --git a/manila/tests/share/drivers/hdfs/test_hdfs_native.py b/manila/tests/share/drivers/hdfs/test_hdfs_native.py index 4933557d16..92b2200d7b 100644 --- a/manila/tests/share/drivers/hdfs/test_hdfs_native.py +++ b/manila/tests/share/drivers/hdfs/test_hdfs_native.py @@ -24,6 +24,7 @@ from manila import context from manila import exception import manila.share.configuration as config import manila.share.drivers.hdfs.hdfs_native as hdfs_native +from manila import ssh_utils from manila import test from manila.tests import fake_share from manila import utils @@ -438,11 +439,13 @@ class HDFSNativeShareDriverTestCase(test.TestCase): ssh.get_transport().is_active = mock.Mock(return_value=True) ssh_pool = mock.Mock() ssh_pool.create = mock.Mock(return_value=ssh) - self.mock_object(utils, 'SSHPool', mock.Mock(return_value=ssh_pool)) + self.mock_object(ssh_utils, + 'SSHPool', + mock.Mock(return_value=ssh_pool)) self.mock_object(processutils, 'ssh_execute', mock.Mock(return_value=ssh_output)) result = self._driver._run_ssh(self.local_ip, cmd_list) - utils.SSHPool.assert_called_once_with( + ssh_utils.SSHPool.assert_called_once_with( self._driver.configuration.hdfs_namenode_ip, self._driver.configuration.hdfs_ssh_port, self._driver.configuration.ssh_conn_timeout, @@ -464,14 +467,16 @@ class HDFSNativeShareDriverTestCase(test.TestCase): ssh.get_transport().is_active = mock.Mock(return_value=True) ssh_pool = mock.Mock() ssh_pool.create = mock.Mock(return_value=ssh) - self.mock_object(utils, 'SSHPool', mock.Mock(return_value=ssh_pool)) + self.mock_object(ssh_utils, + 'SSHPool', + mock.Mock(return_value=ssh_pool)) self.mock_object(processutils, 'ssh_execute', mock.Mock(side_effect=Exception)) self.assertRaises(exception.HDFSException, self._driver._run_ssh, self.local_ip, cmd_list) - utils.SSHPool.assert_called_once_with( + ssh_utils.SSHPool.assert_called_once_with( self._driver.configuration.hdfs_namenode_ip, self._driver.configuration.hdfs_ssh_port, self._driver.configuration.ssh_conn_timeout, diff --git a/manila/tests/share/drivers/hitachi/hnas/test_ssh.py b/manila/tests/share/drivers/hitachi/hnas/test_ssh.py index e68521f89c..fead65e191 100644 --- a/manila/tests/share/drivers/hitachi/hnas/test_ssh.py +++ b/manila/tests/share/drivers/hitachi/hnas/test_ssh.py @@ -23,8 +23,8 @@ import paramiko from manila import exception from manila.share.drivers.hitachi.hnas import ssh +from manila import ssh_utils from manila import test -from manila import utils as mutils CONF = cfg.CONF @@ -1506,7 +1506,7 @@ class HNASSSHTestCase(test.TestCase): mock.Mock(side_effect=[ putils.ProcessExecutionError(stderr=msg), putils.ProcessExecutionError(stderr='Invalid!')])) - self.mock_object(mutils.SSHPool, "item", + self.mock_object(ssh_utils.SSHPool, "item", mock.Mock(return_value=paramiko.SSHClient())) self.mock_object(paramiko.SSHClient, "set_missing_host_key_policy") diff --git a/manila/tests/share/drivers/ibm/test_gpfs.py b/manila/tests/share/drivers/ibm/test_gpfs.py index 57fdd1a36d..58a9dd437c 100644 --- a/manila/tests/share/drivers/ibm/test_gpfs.py +++ b/manila/tests/share/drivers/ibm/test_gpfs.py @@ -26,6 +26,7 @@ from manila import exception import manila.share.configuration as config import manila.share.drivers.ibm.gpfs as gpfs from manila.share import share_types +from manila import ssh_utils from manila import test from manila.tests import fake_share from manila import utils @@ -117,7 +118,9 @@ mmcesnfslsexport:nfsexports:HEADER:version:reserved:reserved:Path:Delegations:Cl expected_cmd = 'fake cmd' ssh_pool = mock.Mock() ssh = mock.Mock() - self.mock_object(utils, 'SSHPool', mock.Mock(return_value=ssh_pool)) + self.mock_object(ssh_utils, + 'SSHPool', + mock.Mock(return_value=ssh_pool)) ssh_pool.item = mock.Mock(return_value=ssh) setattr(ssh, '__enter__', mock.Mock()) setattr(ssh, '__exit__', mock.Mock()) @@ -132,7 +135,9 @@ mmcesnfslsexport:nfsexports:HEADER:version:reserved:reserved:Path:Delegations:Cl cmd_list = ['fake', 'cmd'] ssh_pool = mock.Mock() ssh = mock.Mock() - self.mock_object(utils, 'SSHPool', mock.Mock(return_value=ssh_pool)) + self.mock_object(ssh_utils, + 'SSHPool', + mock.Mock(return_value=ssh_pool)) ssh_pool.item = mock.Mock(return_value=ssh) self.mock_object(self._driver, '_gpfs_ssh_execute') self.assertRaises(exception.GPFSException, diff --git a/manila/tests/share/drivers/inspur/instorage/test_instorage.py b/manila/tests/share/drivers/inspur/instorage/test_instorage.py index 601a1b7587..1c3af3624c 100644 --- a/manila/tests/share/drivers/inspur/instorage/test_instorage.py +++ b/manila/tests/share/drivers/inspur/instorage/test_instorage.py @@ -30,6 +30,7 @@ from manila import exception from manila.share import driver from manila.share.drivers.inspur.instorage import cli_helper from manila.share.drivers.inspur.instorage import instorage +from manila import ssh_utils from manila import test from manila.tests import fake_share from manila import utils as manila_utils @@ -275,7 +276,7 @@ class SSHRunnerTestCase(test.TestCase): def test___call___success(self): mock_csi = self.mock_object(manila_utils, 'check_ssh_injection') mock_sshpool = mock.Mock(return_value=self.fakePool) - self.mock_object(manila_utils, 'SSHPool', mock_sshpool) + self.mock_object(ssh_utils, 'SSHPool', mock_sshpool) mock_se = mock.Mock(return_value='fake_value') self.mock_object(cli_helper.SSHRunner, '_ssh_execute', mock_se) @@ -303,7 +304,7 @@ class SSHRunnerTestCase(test.TestCase): def test___call___ssh_pool_failed(self): mock_csi = self.mock_object(manila_utils, 'check_ssh_injection') mock_sshpool = mock.Mock(side_effect=paramiko.SSHException()) - self.mock_object(manila_utils, 'SSHPool', mock_sshpool) + self.mock_object(ssh_utils, 'SSHPool', mock_sshpool) runner = cli_helper.SSHRunner( '127.0.0.1', '22', 'fakeuser', 'fakepassword' @@ -315,7 +316,7 @@ class SSHRunnerTestCase(test.TestCase): def test___call___ssh_exec_failed(self): mock_csi = self.mock_object(manila_utils, 'check_ssh_injection') mock_sshpool = mock.Mock(return_value=self.fakePool) - self.mock_object(manila_utils, 'SSHPool', mock_sshpool) + self.mock_object(ssh_utils, 'SSHPool', mock_sshpool) exception = processutils.ProcessExecutionError() mock_se = mock.Mock(side_effect=exception) self.mock_object(cli_helper.SSHRunner, '_ssh_execute', mock_se) diff --git a/manila/tests/share/drivers/maprfs/test_maprfs.py b/manila/tests/share/drivers/maprfs/test_maprfs.py index 9cb5773a44..aecbb27fc8 100644 --- a/manila/tests/share/drivers/maprfs/test_maprfs.py +++ b/manila/tests/share/drivers/maprfs/test_maprfs.py @@ -24,6 +24,7 @@ from manila import exception import manila.share.configuration as config from manila.share.drivers.maprfs import driver_util as mapru import manila.share.drivers.maprfs.maprfs_native as maprfs +from manila import ssh_utils from manila import test from manila.tests import fake_share from manila import utils @@ -797,12 +798,14 @@ class MapRFSNativeShareDriverTestCase(test.TestCase): ssh.get_transport().is_active = mock.Mock(return_value=False) ssh_pool = mock.Mock() ssh_pool.create = mock.Mock(return_value=ssh) - self.mock_object(utils, 'SSHPool', mock.Mock(return_value=ssh_pool)) + self.mock_object(ssh_utils, + 'SSHPool', + mock.Mock(return_value=ssh_pool)) self.mock_object(processutils, 'ssh_execute', mock.Mock(return_value=ssh_output)) result = self._driver._maprfs_util._run_ssh( self.local_ip, cmd_list, check_exit_code=False) - utils.SSHPool.assert_called_once_with( + ssh_utils.SSHPool.assert_called_once_with( self._driver.configuration.maprfs_clinode_ip[0], self._driver.configuration.maprfs_ssh_port, self._driver.configuration.ssh_conn_timeout, @@ -824,14 +827,16 @@ class MapRFSNativeShareDriverTestCase(test.TestCase): ssh.get_transport().is_active = mock.Mock(return_value=True) ssh_pool = mock.Mock() ssh_pool.create = mock.Mock(return_value=ssh) - self.mock_object(utils, 'SSHPool', mock.Mock(return_value=ssh_pool)) + self.mock_object(ssh_utils, + 'SSHPool', + mock.Mock(return_value=ssh_pool)) self.mock_object(processutils, 'ssh_execute', mock.Mock( side_effect=exception.ProcessExecutionError)) self.assertRaises(exception.ProcessExecutionError, self._driver._maprfs_util._run_ssh, self.local_ip, cmd_list) - utils.SSHPool.assert_called_once_with( + ssh_utils.SSHPool.assert_called_once_with( self._driver.configuration.maprfs_clinode_ip[0], self._driver.configuration.maprfs_ssh_port, self._driver.configuration.ssh_conn_timeout, diff --git a/manila/tests/share/drivers/test_generic.py b/manila/tests/share/drivers/test_generic.py index 77cbee727d..e85022d6d1 100644 --- a/manila/tests/share/drivers/test_generic.py +++ b/manila/tests/share/drivers/test_generic.py @@ -31,6 +31,7 @@ from manila import exception import manila.share.configuration from manila.share.drivers import generic from manila.share import share_types +from manila import ssh_utils from manila import test from manila.tests import fake_compute from manila.tests import fake_service_instance @@ -1227,14 +1228,16 @@ class GenericShareDriverTestCase(test.TestCase): ssh.get_transport().is_active = mock.Mock(return_value=True) ssh_pool = mock.Mock() ssh_pool.create = mock.Mock(return_value=ssh) - self.mock_object(utils, 'SSHPool', mock.Mock(return_value=ssh_pool)) + self.mock_object(ssh_utils, + 'SSHPool', + mock.Mock(return_value=ssh_pool)) self.mock_object(processutils, 'ssh_execute', mock.Mock(return_value=ssh_output)) self._driver.ssh_connections = {} result = self._driver._ssh_exec(self.server, cmd) - utils.SSHPool.assert_called_once_with( + ssh_utils.SSHPool.assert_called_once_with( self.server['ip'], 22, ssh_conn_timeout, self.server['username'], self.server['password'], self.server['pk_path'], max_size=1) ssh_pool.create.assert_called_once_with() diff --git a/manila/tests/test_ssh_utils.py b/manila/tests/test_ssh_utils.py new file mode 100644 index 0000000000..c35e44d2ef --- /dev/null +++ b/manila/tests/test_ssh_utils.py @@ -0,0 +1,188 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from manila import exception +from manila import ssh_utils +from manila import test +from oslo_utils import uuidutils +import paramiko +from unittest import mock + + +class FakeSock(object): + def settimeout(self, timeout): + pass + + +class FakeTransport(object): + + def __init__(self): + self.active = True + self.sock = FakeSock() + + def set_keepalive(self, timeout): + pass + + def is_active(self): + return self.active + + +class FakeSSHClient(object): + + def __init__(self): + self.id = uuidutils.generate_uuid() + self.transport = FakeTransport() + + def set_missing_host_key_policy(self, policy): + pass + + def connect(self, ip, port=22, username=None, password=None, + key_filename=None, look_for_keys=None, timeout=10, + banner_timeout=10): + pass + + def get_transport(self): + return self.transport + + def close(self): + pass + + def __call__(self, *args, **kwargs): + pass + + +class SSHPoolTestCase(test.TestCase): + """Unit test for SSH Connection Pool.""" + + def test_single_ssh_connect(self): + with mock.patch.object(paramiko, "SSHClient", + mock.Mock(return_value=FakeSSHClient())): + sshpool = ssh_utils.SSHPool("127.0.0.1", 22, 10, "test", + password="test", min_size=1, + max_size=1) + with sshpool.item() as ssh: + first_id = ssh.id + + with sshpool.item() as ssh: + second_id = ssh.id + + self.assertEqual(first_id, second_id) + paramiko.SSHClient.assert_called_once_with() + + def test_create_ssh_with_password(self): + fake_ssh_client = mock.Mock() + ssh_pool = ssh_utils.SSHPool("127.0.0.1", 22, 10, "test", + password="test") + with mock.patch.object(paramiko, "SSHClient", + return_value=fake_ssh_client): + ssh_pool.create() + + fake_ssh_client.connect.assert_called_once_with( + "127.0.0.1", port=22, username="test", + password="test", key_filename=None, look_for_keys=False, + timeout=10, banner_timeout=10) + + def test_create_ssh_with_key(self): + path_to_private_key = "/fakepath/to/privatekey" + fake_ssh_client = mock.Mock() + ssh_pool = ssh_utils.SSHPool("127.0.0.1", 22, 10, + "test", + privatekey="/fakepath/to/privatekey") + with mock.patch.object(paramiko, "SSHClient", + return_value=fake_ssh_client): + ssh_pool.create() + fake_ssh_client.connect.assert_called_once_with( + "127.0.0.1", port=22, username="test", password=None, + key_filename=path_to_private_key, look_for_keys=False, + timeout=10, banner_timeout=10) + + def test_create_ssh_with_nothing(self): + fake_ssh_client = mock.Mock() + ssh_pool = ssh_utils.SSHPool("127.0.0.1", 22, 10, "test") + with mock.patch.object(paramiko, "SSHClient", + return_value=fake_ssh_client): + ssh_pool.create() + fake_ssh_client.connect.assert_called_once_with( + "127.0.0.1", port=22, username="test", password=None, + key_filename=None, look_for_keys=True, + timeout=10, banner_timeout=10) + + def test_create_ssh_error_connecting(self): + attrs = {'connect.side_effect': paramiko.SSHException, } + fake_ssh_client = mock.Mock(**attrs) + ssh_pool = ssh_utils.SSHPool("127.0.0.1", 22, 10, "test") + with mock.patch.object(paramiko, "SSHClient", + return_value=fake_ssh_client): + self.assertRaises(exception.SSHException, ssh_pool.create) + fake_ssh_client.connect.assert_called_once_with( + "127.0.0.1", port=22, username="test", password=None, + key_filename=None, look_for_keys=True, + timeout=10, banner_timeout=10) + + def test_closed_reopend_ssh_connections(self): + with mock.patch.object(paramiko, "SSHClient", + mock.Mock(return_value=FakeSSHClient())): + sshpool = ssh_utils.SSHPool("127.0.0.1", 22, 10, + "test", password="test", + min_size=1, max_size=2) + with sshpool.item() as ssh: + first_id = ssh.id + with sshpool.item() as ssh: + second_id = ssh.id + # Close the connection and test for a new connection + ssh.get_transport().active = False + self.assertEqual(first_id, second_id) + paramiko.SSHClient.assert_called_once_with() + + # Expected new ssh pool + with mock.patch.object(paramiko, "SSHClient", + mock.Mock(return_value=FakeSSHClient())): + with sshpool.item() as ssh: + third_id = ssh.id + self.assertNotEqual(first_id, third_id) + paramiko.SSHClient.assert_called_once_with() + + @mock.patch('builtins.open') + @mock.patch('paramiko.SSHClient') + @mock.patch('os.path.isfile', return_value=True) + def test_sshpool_remove(self, mock_isfile, mock_sshclient, mock_open): + ssh_to_remove = mock.Mock() + mock_sshclient.side_effect = [mock.Mock(), ssh_to_remove, mock.Mock()] + sshpool = ssh_utils.SSHPool("127.0.0.1", 22, 10, + "test", password="test", + min_size=3, max_size=3) + + self.assertIn(ssh_to_remove, list(sshpool.free_items)) + + sshpool.remove(ssh_to_remove) + + self.assertNotIn(ssh_to_remove, list(sshpool.free_items)) + + @mock.patch('builtins.open') + @mock.patch('paramiko.SSHClient') + @mock.patch('os.path.isfile', return_value=True) + def test_sshpool_remove_object_not_in_pool(self, mock_isfile, + mock_sshclient, mock_open): + # create an SSH Client that is not a part of sshpool. + ssh_to_remove = mock.Mock() + mock_sshclient.side_effect = [mock.Mock(), mock.Mock()] + + sshpool = ssh_utils.SSHPool("127.0.0.1", 22, 10, + "test", password="test", + min_size=2, max_size=2) + listBefore = list(sshpool.free_items) + + self.assertNotIn(ssh_to_remove, listBefore) + + sshpool.remove(ssh_to_remove) + + self.assertEqual(listBefore, list(sshpool.free_items)) diff --git a/manila/tests/test_utils.py b/manila/tests/test_utils.py index d881ee978b..b6f312475f 100644 --- a/manila/tests/test_utils.py +++ b/manila/tests/test_utils.py @@ -23,8 +23,6 @@ import ddt from oslo_config import cfg from oslo_utils import encodeutils from oslo_utils import timeutils -from oslo_utils import uuidutils -import paramiko import tenacity from webob import exc @@ -196,171 +194,6 @@ class MonkeyPatchTestCase(test.TestCase): manila.tests.monkey_patch_example.CALLED_FUNCTION) -class FakeSSHClient(object): - - def __init__(self): - self.id = uuidutils.generate_uuid() - self.transport = FakeTransport() - - def set_missing_host_key_policy(self, policy): - pass - - def connect(self, ip, port=22, username=None, password=None, - key_filename=None, look_for_keys=None, timeout=10, - banner_timeout=10): - pass - - def get_transport(self): - return self.transport - - def close(self): - pass - - def __call__(self, *args, **kwargs): - pass - - -class FakeSock(object): - def settimeout(self, timeout): - pass - - -class FakeTransport(object): - - def __init__(self): - self.active = True - self.sock = FakeSock() - - def set_keepalive(self, timeout): - pass - - def is_active(self): - return self.active - - -class SSHPoolTestCase(test.TestCase): - """Unit test for SSH Connection Pool.""" - - def test_single_ssh_connect(self): - with mock.patch.object(paramiko, "SSHClient", - mock.Mock(return_value=FakeSSHClient())): - sshpool = utils.SSHPool("127.0.0.1", 22, 10, "test", - password="test", min_size=1, max_size=1) - with sshpool.item() as ssh: - first_id = ssh.id - - with sshpool.item() as ssh: - second_id = ssh.id - - self.assertEqual(first_id, second_id) - paramiko.SSHClient.assert_called_once_with() - - def test_create_ssh_with_password(self): - fake_ssh_client = mock.Mock() - ssh_pool = utils.SSHPool("127.0.0.1", 22, 10, "test", - password="test") - with mock.patch.object(paramiko, "SSHClient", - return_value=fake_ssh_client): - ssh_pool.create() - - fake_ssh_client.connect.assert_called_once_with( - "127.0.0.1", port=22, username="test", - password="test", key_filename=None, look_for_keys=False, - timeout=10, banner_timeout=10) - - def test_create_ssh_with_key(self): - path_to_private_key = "/fakepath/to/privatekey" - fake_ssh_client = mock.Mock() - ssh_pool = utils.SSHPool("127.0.0.1", 22, 10, "test", - privatekey="/fakepath/to/privatekey") - with mock.patch.object(paramiko, "SSHClient", - return_value=fake_ssh_client): - ssh_pool.create() - fake_ssh_client.connect.assert_called_once_with( - "127.0.0.1", port=22, username="test", password=None, - key_filename=path_to_private_key, look_for_keys=False, - timeout=10, banner_timeout=10) - - def test_create_ssh_with_nothing(self): - fake_ssh_client = mock.Mock() - ssh_pool = utils.SSHPool("127.0.0.1", 22, 10, "test") - with mock.patch.object(paramiko, "SSHClient", - return_value=fake_ssh_client): - ssh_pool.create() - fake_ssh_client.connect.assert_called_once_with( - "127.0.0.1", port=22, username="test", password=None, - key_filename=None, look_for_keys=True, - timeout=10, banner_timeout=10) - - def test_create_ssh_error_connecting(self): - attrs = {'connect.side_effect': paramiko.SSHException, } - fake_ssh_client = mock.Mock(**attrs) - ssh_pool = utils.SSHPool("127.0.0.1", 22, 10, "test") - with mock.patch.object(paramiko, "SSHClient", - return_value=fake_ssh_client): - self.assertRaises(exception.SSHException, ssh_pool.create) - fake_ssh_client.connect.assert_called_once_with( - "127.0.0.1", port=22, username="test", password=None, - key_filename=None, look_for_keys=True, - timeout=10, banner_timeout=10) - - def test_closed_reopend_ssh_connections(self): - with mock.patch.object(paramiko, "SSHClient", - mock.Mock(return_value=FakeSSHClient())): - sshpool = utils.SSHPool("127.0.0.1", 22, 10, "test", - password="test", min_size=1, max_size=2) - with sshpool.item() as ssh: - first_id = ssh.id - with sshpool.item() as ssh: - second_id = ssh.id - # Close the connection and test for a new connection - ssh.get_transport().active = False - self.assertEqual(first_id, second_id) - paramiko.SSHClient.assert_called_once_with() - - # Expected new ssh pool - with mock.patch.object(paramiko, "SSHClient", - mock.Mock(return_value=FakeSSHClient())): - with sshpool.item() as ssh: - third_id = ssh.id - self.assertNotEqual(first_id, third_id) - paramiko.SSHClient.assert_called_once_with() - - @mock.patch('builtins.open') - @mock.patch('paramiko.SSHClient') - @mock.patch('os.path.isfile', return_value=True) - def test_sshpool_remove(self, mock_isfile, mock_sshclient, mock_open): - ssh_to_remove = mock.Mock() - mock_sshclient.side_effect = [mock.Mock(), ssh_to_remove, mock.Mock()] - sshpool = utils.SSHPool("127.0.0.1", 22, 10, "test", password="test", - min_size=3, max_size=3) - - self.assertIn(ssh_to_remove, list(sshpool.free_items)) - - sshpool.remove(ssh_to_remove) - - self.assertNotIn(ssh_to_remove, list(sshpool.free_items)) - - @mock.patch('builtins.open') - @mock.patch('paramiko.SSHClient') - @mock.patch('os.path.isfile', return_value=True) - def test_sshpool_remove_object_not_in_pool(self, mock_isfile, - mock_sshclient, mock_open): - # create an SSH Client that is not a part of sshpool. - ssh_to_remove = mock.Mock() - mock_sshclient.side_effect = [mock.Mock(), mock.Mock()] - - sshpool = utils.SSHPool("127.0.0.1", 22, 10, "test", password="test", - min_size=2, max_size=2) - listBefore = list(sshpool.free_items) - - self.assertNotIn(ssh_to_remove, listBefore) - - sshpool.remove(ssh_to_remove) - - self.assertEqual(listBefore, list(sshpool.free_items)) - - @ddt.ddt class CidrToNetmaskTestCase(test.TestCase): """Unit test for cidr to netmask.""" diff --git a/manila/utils.py b/manila/utils.py index e24e8a1eaa..a2833efc51 100644 --- a/manila/utils.py +++ b/manila/utils.py @@ -20,7 +20,6 @@ import contextlib import functools import inspect -import os import pyclbr import re import shutil @@ -29,7 +28,6 @@ import tempfile import tenacity import time -from eventlet import pools import logging import netaddr from oslo_concurrency import lockutils @@ -38,10 +36,8 @@ from oslo_config import cfg from oslo_log import log from oslo_utils import importutils from oslo_utils import netutils -from oslo_utils.secretutils import md5 from oslo_utils import strutils from oslo_utils import timeutils -import paramiko from webob import exc @@ -62,21 +58,6 @@ _ISO8601_TIME_FORMAT = '%Y-%m-%dT%H:%M:%S' synchronized = lockutils.synchronized_with_prefix('manila-') -def get_fingerprint(self): - """Patch paramiko - - This method needs to be patched to allow paramiko to work under FIPS. - Until the patch to do this merges, patch paramiko here. - - TODO(carloss) Remove this when paramiko is patched. - See https://github.com/paramiko/paramiko/pull/1928 - """ - return md5(self.asbytes(), usedforsecurity=False).digest() - - -paramiko.pkey.PKey.get_fingerprint = get_fingerprint - - def isotime(at=None, subsecond=False): """Stringify time in ISO 8601 format.""" @@ -115,85 +96,6 @@ def execute(*cmd, **kwargs): return processutils.execute(*cmd, **kwargs) -class SSHPool(pools.Pool): - """A simple eventlet pool to hold ssh connections.""" - - def __init__(self, ip, port, conn_timeout, login, password=None, - privatekey=None, *args, **kwargs): - self.ip = ip - self.port = port - self.login = login - self.password = password - self.conn_timeout = conn_timeout if conn_timeout else None - self.path_to_private_key = privatekey - super(SSHPool, self).__init__(*args, **kwargs) - - def create(self): # pylint: disable=method-hidden - ssh = paramiko.SSHClient() - ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) - look_for_keys = True - if self.path_to_private_key: - self.path_to_private_key = os.path.expanduser( - self.path_to_private_key) - look_for_keys = False - elif self.password: - look_for_keys = False - try: - LOG.debug("ssh.connect: ip: %s, port: %s, look_for_keys: %s, " - "timeout: %s, banner_timeout: %s", - self.ip, - self.port, - look_for_keys, - self.conn_timeout, - self.conn_timeout) - ssh.connect(self.ip, - port=self.port, - username=self.login, - password=self.password, - key_filename=self.path_to_private_key, - look_for_keys=look_for_keys, - timeout=self.conn_timeout, - banner_timeout=self.conn_timeout) - if self.conn_timeout: - transport = ssh.get_transport() - transport.set_keepalive(self.conn_timeout) - return ssh - except Exception as e: - msg = _("Check whether private key or password are correctly " - "set. Error connecting via ssh: %s") % e - LOG.error(msg) - raise exception.SSHException(msg) - - def get(self): - """Return an item from the pool, when one is available. - - This may cause the calling greenthread to block. Check if a - connection is active before returning it. For dead connections - create and return a new connection. - """ - if self.free_items: - conn = self.free_items.popleft() - if conn: - if conn.get_transport().is_active(): - return conn - else: - conn.close() - return self.create() - if self.current_size < self.max_size: - created = self.create() - self.current_size += 1 - return created - return self.channel.get() - - def remove(self, ssh): - """Close an ssh client and remove it from free_items.""" - ssh.close() - if ssh in self.free_items: - self.free_items.remove(ssh) - if self.current_size > 0: - self.current_size -= 1 - - def check_ssh_injection(cmd_list): ssh_injection_pattern = ['`', '$', '|', '||', ';', '&', '&&', '>', '>>', '<']