Merge pull request #63050 from barneysowood/default-disable-salt-api-…

…clients

Add option to enable/disable netapi clients

changelog/63050.changed

@@ -0,0 +1,5 @@
+netapi_enable_clients option to allow enabling/disabling of clients in salt-api.
+By default all clients will now be disabled. Users of salt-api will need
+to update their master config to enable the clients that they use. Not adding
+the netapi_enable_clients option with required clients to the master config will 
+disable salt-api.

conf/master

@@ -1340,3 +1340,6 @@
 ############################################
 # Allow the raw_shell parameter to be used when calling Salt SSH client via API
 #netapi_allow_raw_shell: True
+
+# Set a list of clients to enable in in the API
+#netapi_enable_clients: []

doc/ref/configuration/master.rst

@@ -5099,6 +5099,41 @@ Used by ``salt-api`` for the master requests timeout.
 
     rest_timeout: 300
 
+.. conf_master:: netapi_disable_clients
+
+``netapi_enable_clients``
+--------------------------
+
+.. versionadded:: 3006.0
+
+Default: ``[]``
+
+Used by ``salt-api`` to enable access to the listed clients. Unless a
+client is addded to this list, requests will be rejected before
+authentication is attempted or processing of the low state occurs.
+
+This can be used to only expose the required functionality via
+``salt-api``.
+
+Configuration with all possible clients enabled:
+
+.. code-block:: yaml
+
+    netapi_enable_clients:
+      - local
+      - local_async
+      - local_batch
+      - local_subset
+      - runner
+      - runner_async
+      - ssh
+      - wheel
+      - wheel_async
+
+.. note::
+
+    Enabling all clients is not recommended - only enable the
+    clients that provide the functionality required.
 
 .. _syndic-server-settings:
 

doc/topics/netapi/index.rst

@@ -37,6 +37,18 @@ values that are mapped to function arguments. This allows calling functions
 simply by creating a data structure. (And this is exactly how much of Salt's
 own internals work!)
 
+The :conf_master:`netapi_enable_clients` list in the master config sets which
+clients are available. It is recommended to only enable the clients required
+to complete the tasks needed to reduce the amount of Salt functionality exposed
+via the netapi. Enabling the local clients will provide the same functionality as 
+the :command:`salt` command.
+
+.. admonition:: :conf_master:`netapi_enable_clients`
+
+    Prior to Salt's 3006.0 release all clients were enabled and it was not possible
+    to disable clients individually.
+
+
 .. autoclass:: salt.netapi.NetapiClient
     :members: local, local_async, local_subset, ssh, runner, runner_async,
         wheel, wheel_async

doc/topics/releases/3006.rst

@@ -28,6 +28,18 @@ supported versions. See
 for more information.
 
 
+All salt-api functionality disabled by default
+----------------------------------------------
+
+All netapi clients, which provide the functionality to salt-api, will now
+be disabled by default. If you use the salt-api you must add the new
+`netapi_enable_clients` option to your salt master config. This is
+a breaking change and the salt-api will not function without this
+new configuration option.
+See `Netapi module <https://docs.saltproject.io/en/latest/topics/netapi/index.html#introduction-to-netapi-modules>`
+for more information.
+
+
 How do I migrate to the onedir packages?
 ----------------------------------------
 The migration path from the classic, non-onedir packages to the onedir packages

salt/config/__init__.py

@@ -954,6 +954,8 @@ def _gather_buffer_space():
         # Allow raw_shell option when using the ssh
         # client via the Salt API
         "netapi_allow_raw_shell": bool,
+        # Enable clients in the Salt API
+        "netapi_enable_clients": list,
         "disabled_requisites": (str, list),
         "global_state_conditions": (type(None), dict),
         # Feature flag config
@@ -1618,6 +1620,7 @@ def _gather_buffer_space():
         "pass_strict_fetch": False,
         "pass_gnupghome": "",
         "pass_dir": "",
+        "netapi_enable_clients": [],
     }
 )
 

salt/netapi/__init__.py

@@ -157,6 +157,13 @@ def run(self, low):
                 "Invalid client specified: '{}'".format(low.get("client"))
             )
 
+        if low.get("client") not in self.opts.get("netapi_enable_clients"):
+            raise salt.exceptions.SaltInvocationError(
+                "Client disabled: '{}'. Add to 'netapi_enable_clients' master config option to enable.".format(
+                    low.get("client")
+                )
+            )
+
         if not ("token" in low or "eauth" in low):
             raise salt.exceptions.EauthAuthenticationError(
                 "No authentication credentials given"

tests/pytests/functional/netapi/rest_cherrypy/conftest.py

@@ -11,6 +11,7 @@
 @pytest.fixture
 def client_config(client_config, netapi_port):
     client_config["rest_cherrypy"] = {"port": netapi_port, "debug": True}
+    client_config["netapi_enable_clients"] = ["local"]
     return client_config
 
 

tests/pytests/functional/netapi/rest_tornado/conftest.py

@@ -6,6 +6,12 @@
 @pytest.fixture
 def client_config(client_config, netapi_port):
     client_config["rest_tornado"] = {"port": netapi_port}
+    client_config["netapi_enable_clients"] = [
+        "local",
+        "local_async",
+        "runner",
+        "runner_async",
+    ]
     return client_config
 
 

tests/pytests/integration/netapi/rest_cherrypy/conftest.py

@@ -10,6 +10,7 @@
 @pytest.fixture
 def client_config(client_config, netapi_port):
     client_config["rest_cherrypy"] = {"port": netapi_port, "debug": True}
+    client_config["netapi_enable_clients"] = ["local", "runner"]
     return client_config
 
 

tests/pytests/integration/netapi/rest_tornado/conftest.py

@@ -7,6 +7,12 @@
 @pytest.fixture
 def client_config(client_config, netapi_port):
     client_config["rest_tornado"] = {"port": netapi_port}
+    client_config["netapi_enable_clients"] = [
+        "local",
+        "local_async",
+        "runner",
+        "runner_async",
+    ]
     return client_config
 
 

tests/pytests/integration/netapi/test_client.py

@@ -1,7 +1,8 @@
 import pytest
 
 import salt.netapi
-from salt.exceptions import EauthAuthenticationError
+from salt.exceptions import EauthAuthenticationError, SaltInvocationError
+from tests.support.mock import patch
 
 
 @pytest.fixture
@@ -13,15 +14,17 @@ def client(salt_minion, salt_sub_minion, client_config):
 def test_local(client, auth_creds, salt_minion, salt_sub_minion):
     low = {"client": "local", "tgt": "*", "fun": "test.ping", **auth_creds}
 
-    ret = client.run(low)
+    with patch.dict(client.opts, {"netapi_enable_clients": ["local"]}):
+        ret = client.run(low)
     assert ret == {salt_minion.id: True, salt_sub_minion.id: True}
 
 
 @pytest.mark.slow_test
 def test_local_batch(client, auth_creds, salt_minion, salt_sub_minion):
     low = {"client": "local_batch", "tgt": "*", "fun": "test.ping", **auth_creds}
 
-    ret = client.run(low)
+    with patch.dict(client.opts, {"netapi_enable_clients": ["local_batch"]}):
+        ret = client.run(low)
     assert ret
     # local_batch returns a generator
     ret = list(ret)
@@ -33,23 +36,63 @@ def test_local_batch(client, auth_creds, salt_minion, salt_sub_minion):
 def test_local_async(client, auth_creds, salt_minion, salt_sub_minion):
     low = {"client": "local_async", "tgt": "*", "fun": "test.ping", **auth_creds}
 
-    ret = client.run(low)
+    with patch.dict(client.opts, {"netapi_enable_clients": ["local_async"]}):
+        ret = client.run(low)
 
     assert "jid" in ret
     assert sorted(ret["minions"]) == sorted([salt_minion.id, salt_sub_minion.id])
 
 
 def test_local_unauthenticated(client):
     low = {"client": "local", "tgt": "*", "fun": "test.ping"}
-    with pytest.raises(EauthAuthenticationError):
-        client.run(low)
+
+    with patch.dict(client.opts, {"netapi_enable_clients": ["local"]}):
+        with pytest.raises(EauthAuthenticationError):
+            client.run(low)
+
+
+def test_local_disabled(client, auth_creds):
+    low = {"client": "local", "tgt": "*", "fun": "test.ping", **auth_creds}
+
+    ret = None
+    with pytest.raises(SaltInvocationError):
+        ret = client.run(low)
+
+    assert ret is None
+
+
+def test_local_batch_disabled(client, auth_creds):
+    low = {"client": "local_batch", "tgt": "*", "fun": "test.ping", **auth_creds}
+
+    ret = None
+    with pytest.raises(SaltInvocationError):
+        ret = client.run(low)
+
+    assert ret is None
+
+
+def test_local_subset_disabled(client, auth_creds):
+    low = {
+        "client": "local_subset",
+        "tgt": "*",
+        "fun": "test.ping",
+        "subset": 1,
+        **auth_creds,
+    }
+
+    ret = None
+    with pytest.raises(SaltInvocationError):
+        ret = client.run(low)
+
+    assert ret is None
 
 
 @pytest.mark.slow_test
 def test_wheel(client, auth_creds):
     low = {"client": "wheel", "fun": "key.list_all", **auth_creds}
 
-    ret = client.run(low)
+    with patch.dict(client.opts, {"netapi_enable_clients": ["wheel"]}):
+        ret = client.run(low)
 
     assert "tag" in ret
     assert "data" in ret
@@ -64,19 +107,53 @@ def test_wheel(client, auth_creds):
 def test_wheel_async(client, auth_creds):
     low = {"client": "wheel_async", "fun": "key.list_all", **auth_creds}
 
-    ret = client.run(low)
+    with patch.dict(client.opts, {"netapi_enable_clients": ["wheel_async"]}):
+        ret = client.run(low)
     assert "jid" in ret
     assert "tag" in ret
 
 
 def test_wheel_unauthenticated(client):
     low = {"client": "wheel", "tgt": "*", "fun": "test.ping"}
 
-    with pytest.raises(EauthAuthenticationError):
-        client.run(low)
+    with patch.dict(client.opts, {"netapi_enable_clients": ["wheel"]}):
+        with pytest.raises(EauthAuthenticationError):
+            client.run(low)
+
+
+def test_wheel_disabled(client, auth_creds):
+    low = {"client": "wheel", "tgt": "*", "fun": "test.ping", **auth_creds}
+
+    ret = None
+    with pytest.raises(SaltInvocationError):
+        ret = client.run(low)
+
+    assert ret is None
+
+
+def test_wheel_async_disabled(client, auth_creds):
+    low = {"client": "wheel_async", "tgt": "*", "fun": "test.ping", **auth_creds}
+
+    ret = None
+    with pytest.raises(SaltInvocationError):
+        ret = client.run(low)
+
+    assert ret is None
 
 
 def test_runner_unauthenticated(client):
     low = {"client": "runner", "tgt": "*", "fun": "test.ping"}
-    with pytest.raises(EauthAuthenticationError):
-        client.run(low)
+
+    with patch.dict(client.opts, {"netapi_enable_clients": ["runner"]}):
+        with pytest.raises(EauthAuthenticationError):
+            client.run(low)
+
+
+def test_runner_disabled(client, auth_creds):
+    low = {"client": "runner", "tgt": "*", "fun": "test.ping", **auth_creds}
+
+    ret = None
+    with pytest.raises(SaltInvocationError):
+        ret = client.run(low)
+
+    assert ret is None

tests/pytests/integration/netapi/test_ssh_client.py

@@ -1,7 +1,7 @@
 import pytest
 
 import salt.netapi
-from salt.exceptions import EauthAuthenticationError
+from salt.exceptions import EauthAuthenticationError, SaltInvocationError
 from tests.support.helpers import SaveRequestsPostHandler, Webserver
 from tests.support.mock import patch
 
@@ -11,6 +11,12 @@
 ]
 
 
+@pytest.fixture
+def client_config(client_config):
+    client_config["netapi_enable_clients"] = ["ssh"]
+    return client_config
+
+
 @pytest.fixture
 def client(client_config, salt_minion):
     return salt.netapi.NetapiClient(client_config)
@@ -77,6 +83,7 @@ def test_ssh(client, auth_creds, salt_ssh_roster_file, rosters_dir, ssh_priv_key
 
 def test_ssh_unauthenticated(client):
     low = {"client": "ssh", "tgt": "localhost", "fun": "test.ping"}
+
     with pytest.raises(EauthAuthenticationError):
         client.run(low)
 
@@ -117,6 +124,17 @@ def test_ssh_authenticated_raw_shell_disabled(client, tmp_path):
     assert badfile.exists() is False
 
 
+def test_ssh_disabled(client, auth_creds):
+    low = {"client": "ssh", "tgt": "localhost", "fun": "test.ping", **auth_creds}
+
+    ret = None
+    with patch.dict(client.opts, {"netapi_enable_clients": []}):
+        with pytest.raises(SaltInvocationError):
+            ret = client.run(low)
+
+    assert ret is None
+
+
 def test_shell_inject_ssh_priv(
     client, salt_ssh_roster_file, rosters_dir, tmp_path, salt_auto_account
 ):