From 95f38ddf067b9c52654395a217afea988e44a54f Mon Sep 17 00:00:00 2001
From: Jochen Breuer <jbreuer@suse.de>
Date: Wed, 19 Feb 2020 14:37:05 +0100
Subject: [PATCH] various netapi fixes and tests

---
 conf/master                             |   6 ++
 salt/config/__init__.py                 |   6 +-
 salt/netapi/__init__.py                 |   7 +-
 tests/integration/netapi/test_client.py | 150 +++++++++++++++++++++++++++++++-
 tests/support/helpers.py                |  19 ++++
 5 files changed, 185 insertions(+), 3 deletions(-)

diff --git a/conf/master b/conf/master
index 06bed3ea44..349d971414 100644
--- a/conf/master
+++ b/conf/master
@@ -1291,3 +1291,9 @@ syndic_user: salt
 # use OS defaults, typically 75 seconds on Linux, see
 # /proc/sys/net/ipv4/tcp_keepalive_intvl.
 #tcp_keepalive_intvl: -1
+
+
+#####         NetAPI settings          #####
+############################################
+# Allow the raw_shell parameter to be used when calling Salt SSH client via API
+#netapi_allow_raw_shell: True
diff --git a/salt/config/__init__.py b/salt/config/__init__.py
index 5d0c18b5d1..dc257ff8b8 100644
--- a/salt/config/__init__.py
+++ b/salt/config/__init__.py
@@ -1216,6 +1216,10 @@ VALID_OPTS = {
     # Use Adler32 hashing algorithm for server_id (default False until Sodium, "adler32" after)
     # Possible values are: False, adler32, crc32
     'server_id_use_crc': (bool, six.string_types),
+
+    # Allow raw_shell option when using the ssh
+    # client via the Salt API
+    'netapi_allow_raw_shell': bool,
 }
 
 # default configurations
@@ -1869,9 +1873,9 @@ DEFAULT_MASTER_OPTS = {
     'auth_events': True,
     'minion_data_cache_events': True,
     'enable_ssh_minions': False,
+    'netapi_allow_raw_shell': False,
 }
 
-
 # ----- Salt Proxy Minion Configuration Defaults ----------------------------------->
 # These are merged with DEFAULT_MINION_OPTS since many of them also apply here.
 DEFAULT_PROXY_MINION_OPTS = {
diff --git a/salt/netapi/__init__.py b/salt/netapi/__init__.py
index 43b6e943a7..31a24bb420 100644
--- a/salt/netapi/__init__.py
+++ b/salt/netapi/__init__.py
@@ -71,10 +71,15 @@ class NetapiClient(object):
             raise salt.exceptions.SaltInvocationError(
                     'Invalid client specified: \'{0}\''.format(low.get('client')))
 
-        if not ('token' in low or 'eauth' in low) and low['client'] != 'ssh':
+        if not ('token' in low or 'eauth' in low):
             raise salt.exceptions.EauthAuthenticationError(
                     'No authentication credentials given')
 
+        if low.get('raw_shell') and \
+                not self.opts.get('netapi_allow_raw_shell'):
+            raise salt.exceptions.EauthAuthenticationError(
+                    'Raw shell option not allowed.')
+
         l_fun = getattr(self, low['client'])
         f_call = salt.utils.args.format_call(l_fun, low)
         return l_fun(*f_call.get('args', ()), **f_call.get('kwargs', {}))
diff --git a/tests/integration/netapi/test_client.py b/tests/integration/netapi/test_client.py
index 503bbaf335..a886563e3d 100644
--- a/tests/integration/netapi/test_client.py
+++ b/tests/integration/netapi/test_client.py
@@ -2,17 +2,32 @@
 
 # Import Python libs
 from __future__ import absolute_import, print_function, unicode_literals
+import logging
 import os
 import time
 
 # Import Salt Testing libs
-from tests.support.paths import TMP_CONF_DIR
+from tests.support.paths import TMP_CONF_DIR, TMP
+from tests.support.runtests import RUNTIME_VARS
 from tests.support.unit import TestCase, skipIf
+from tests.support.mock import patch
+from tests.support.case import SSHCase
+from tests.support.helpers import (
+    Webserver,
+    SaveRequestsPostHandler,
+    requires_sshd_server
+)
 
 # Import Salt libs
 import salt.config
 import salt.netapi
 
+from salt.exceptions import (
+    EauthAuthenticationError
+)
+
+log = logging.getLogger(__name__)
+
 
 class NetapiClientTest(TestCase):
     eauth_creds = {
@@ -74,6 +89,12 @@ class NetapiClientTest(TestCase):
             pass
         self.assertEqual(ret, {'minions': sorted(['minion', 'sub_minion'])})
 
+    def test_local_unauthenticated(self):
+        low = {'client': 'local', 'tgt': '*', 'fun': 'test.ping'}
+
+        with self.assertRaises(EauthAuthenticationError) as excinfo:
+            ret = self.netapi.run(low)
+
     def test_wheel(self):
         low = {'client': 'wheel', 'fun': 'key.list_all'}
         low.update(self.eauth_creds)
@@ -107,6 +128,12 @@ class NetapiClientTest(TestCase):
         self.assertIn('jid', ret)
         self.assertIn('tag', ret)
 
+    def test_wheel_unauthenticated(self):
+        low = {'client': 'wheel', 'tgt': '*', 'fun': 'test.ping'}
+
+        with self.assertRaises(EauthAuthenticationError) as excinfo:
+            ret = self.netapi.run(low)
+
     @skipIf(True, 'This is not testing anything. Skipping for now.')
     def test_runner(self):
         # TODO: fix race condition in init of event-- right now the event class
@@ -125,3 +152,124 @@ class NetapiClientTest(TestCase):
         low.update(self.eauth_creds)
 
         ret = self.netapi.run(low)
+
+    def test_runner_unauthenticated(self):
+        low = {'client': 'runner', 'tgt': '*', 'fun': 'test.ping'}
+
+        with self.assertRaises(EauthAuthenticationError) as excinfo:
+            ret = self.netapi.run(low)
+
+
+@requires_sshd_server
+class NetapiSSHClientTest(SSHCase):
+    eauth_creds = {
+        'username': 'saltdev_auto',
+        'password': 'saltdev',
+        'eauth': 'auto',
+    }
+
+    def setUp(self):
+        '''
+        Set up a NetapiClient instance
+        '''
+        opts = salt.config.client_config(os.path.join(TMP_CONF_DIR, 'master'))
+        self.netapi = salt.netapi.NetapiClient(opts)
+        self.priv_file = os.path.join(RUNTIME_VARS.TMP_CONF_DIR, 'key_test')
+        self.rosters = os.path.join(RUNTIME_VARS.TMP_CONF_DIR)
+
+        self.priv_file = os.path.join(RUNTIME_VARS.TMP_CONF_DIR, 'key_test')
+        self.rosters = os.path.join(RUNTIME_VARS.TMP_CONF_DIR)
+
+        # Initialize salt-ssh
+        self.run_function('test.ping')
+
+    def tearDown(self):
+        del self.netapi
+
+    @classmethod
+    def setUpClass(cls):
+        cls.post_webserver = Webserver(handler=SaveRequestsPostHandler)
+        cls.post_webserver.start()
+        cls.post_web_root = cls.post_webserver.web_root
+        cls.post_web_handler = cls.post_webserver.handler
+
+    @classmethod
+    def tearDownClass(cls):
+        cls.post_webserver.stop()
+        del cls.post_webserver
+
+    def test_ssh(self):
+        low = {'client': 'ssh',
+               'tgt': 'localhost',
+               'fun': 'test.ping',
+               'ignore_host_keys': True,
+               'roster_file': 'roster',
+               'rosters': [self.rosters],
+               'ssh_priv': self.priv_file}
+
+        low.update(self.eauth_creds)
+
+        ret = self.netapi.run(low)
+
+        self.assertIn('localhost', ret)
+        self.assertIn('return', ret['localhost'])
+        self.assertEqual(ret['localhost']['return'], True)
+        self.assertEqual(ret['localhost']['id'], 'localhost')
+        self.assertEqual(ret['localhost']['fun'], 'test.ping')
+
+    def test_ssh_unauthenticated(self):
+        low = {'client': 'ssh', 'tgt': 'localhost', 'fun': 'test.ping'}
+
+        with self.assertRaises(EauthAuthenticationError) as excinfo:
+            ret = self.netapi.run(low)
+
+    def test_ssh_unauthenticated_raw_shell_curl(self):
+
+        fun = '-o ProxyCommand curl {0}'.format(self.post_web_root)
+        low = {'client': 'ssh',
+               'tgt': 'localhost',
+               'fun': fun,
+               'raw_shell': True}
+
+        ret = None
+        with self.assertRaises(EauthAuthenticationError) as excinfo:
+            ret = self.netapi.run(low)
+
+        self.assertEqual(self.post_web_handler.received_requests, [])
+        self.assertEqual(ret, None)
+
+    def test_ssh_unauthenticated_raw_shell_touch(self):
+
+        badfile = os.path.join(TMP, 'badfile.txt')
+        fun = '-o ProxyCommand touch {0}'.format(badfile)
+        low = {'client': 'ssh',
+               'tgt': 'localhost',
+               'fun': fun,
+               'raw_shell': True}
+
+        ret = None
+        with self.assertRaises(EauthAuthenticationError) as excinfo:
+            ret = self.netapi.run(low)
+
+        self.assertEqual(ret, None)
+        self.assertFalse(os.path.exists('badfile.txt'))
+
+    def test_ssh_authenticated_raw_shell_disabled(self):
+
+        badfile = os.path.join(TMP, 'badfile.txt')
+        fun = '-o ProxyCommand touch {0}'.format(badfile)
+        low = {'client': 'ssh',
+               'tgt': 'localhost',
+               'fun': fun,
+               'raw_shell': True}
+
+        low.update(self.eauth_creds)
+
+        ret = None
+        with patch.dict(self.netapi.opts,
+                        {'netapi_allow_raw_shell': False}):
+            with self.assertRaises(EauthAuthenticationError) as excinfo:
+                ret = self.netapi.run(low)
+
+        self.assertEqual(ret, None)
+        self.assertFalse(os.path.exists('badfile.txt'))
diff --git a/tests/support/helpers.py b/tests/support/helpers.py
index 626da6a069..e5ca5918c9 100644
--- a/tests/support/helpers.py
+++ b/tests/support/helpers.py
@@ -1582,6 +1582,25 @@ class Webserver(object):
         self.server_thread.join()
 
 
+class SaveRequestsPostHandler(tornado.web.RequestHandler):
+    '''
+    Save all requests sent to the server.
+    '''
+    received_requests = []
+
+    def post(self, *args):  # pylint: disable=arguments-differ
+        '''
+        Handle the post
+        '''
+        self.received_requests.append(self.request)
+
+    def data_received(self):  # pylint: disable=arguments-differ
+        '''
+        Streaming not used for testing
+        '''
+        raise NotImplementedError()
+
+
 def win32_kill_process_tree(pid, sig=signal.SIGTERM, include_parent=True,
         timeout=None, on_terminate=None):
     '''
-- 
2.16.4


