[3007.x] Fix master pull socket permissions

changelog/66228.fixed.md

@@ -0,0 +1 @@
+Make sure the master_event_pub.ipc file has correct reed/write permissions for salt group.

salt/transport/base.py

@@ -211,6 +211,14 @@ def ipc_publish_client(node, opts, io_loop):
 
 
 def ipc_publish_server(node, opts):
+    """
+    Create an IPC publish server.
+
+    With the exception of a master's pull_path, all ipc path permission have
+    user read/write permissions. On a master the ipc publish server's pull_path
+    permissions are also group read/write. This is done to facilitate non root
+    users running the salt cli to execute jobs on a master.
+    """
     # Default to TCP for now
     kwargs = {"transport": "tcp", "ssl": None}
     if opts["ipc_mode"] == "tcp":
@@ -233,6 +241,7 @@ def ipc_publish_server(node, opts):
             kwargs.update(
                 pub_path=os.path.join(opts["sock_dir"], "master_event_pub.ipc"),
                 pull_path=os.path.join(opts["sock_dir"], "master_event_pull.ipc"),
+                pub_path_perms=0o660,
             )
         else:
             id_hash = _minion_hash(

salt/transport/tcp.py

@@ -10,6 +10,7 @@
 import errno
 import logging
 import multiprocessing
+import os
 import queue
 import select
 import socket
@@ -1327,6 +1328,8 @@ def __init__(
         pull_host=None,
         pull_port=None,
         pull_path=None,
+        pull_path_perms=0o600,
+        pub_path_perms=0o600,
         ssl=None,
     ):
         self.opts = opts
@@ -1337,6 +1340,8 @@ def __init__(
         self.pull_host = pull_host
         self.pull_port = pull_port
         self.pull_path = pull_path
+        self.pull_path_perms = pull_path_perms
+        self.pub_path_perms = pub_path_perms
         self.ssl = ssl
 
     @property
@@ -1355,6 +1360,8 @@ def __getstate__(self):
             "pull_host": self.pull_host,
             "pull_port": self.pull_port,
             "pull_path": self.pull_path,
+            "pub_path_perms": self.pub_path_perms,
+            "pull_path_perms": self.pull_path_perms,
         }
 
     def publish_daemon(
@@ -1406,7 +1413,9 @@ async def publisher(
             log.debug(
                 "Publish server binding pub to %s ssl=%r", self.pub_path, self.ssl
             )
-            sock = tornado.netutil.bind_unix_socket(self.pub_path)
+            with salt.utils.files.set_umask(0o177):
+                sock = tornado.netutil.bind_unix_socket(self.pub_path)
+                os.chmod(self.pub_path, self.pub_path_perms)
         else:
             log.debug(
                 "Publish server binding pub to %s:%s ssl=%r",
@@ -1446,6 +1455,7 @@ async def publisher(
         # Securely create socket
         with salt.utils.files.set_umask(0o177):
             self.pull_sock.start()
+            os.chmod(self.pull_path, self.pull_path_perms)
 
     def pre_fork(self, process_manager):
         """

salt/transport/ws.py

@@ -1,6 +1,7 @@
 import asyncio
 import logging
 import multiprocessing
+import os
 import socket
 import time
 import warnings
@@ -259,6 +260,8 @@ def __init__(
         pull_host=None,
         pull_port=None,
         pull_path=None,
+        pull_path_perms=0o600,
+        pub_path_perms=0o600,
         ssl=None,
     ):
         self.opts = opts
@@ -268,6 +271,8 @@ def __init__(
         self.pull_host = pull_host
         self.pull_port = pull_port
         self.pull_path = pull_path
+        self.pull_path_perms = pull_path_perms
+        self.pub_path_perms = pub_path_perms
         self.ssl = ssl
         self.clients = set()
         self._run = None
@@ -291,6 +296,8 @@ def __getstate__(self):
             "pull_host": self.pull_host,
             "pull_port": self.pull_port,
             "pull_path": self.pull_path,
+            "pull_path_perms": self.pull_path_perms,
+            "pub_path_perms": self.pub_path_perms,
         }
 
     def publish_daemon(
@@ -338,8 +345,11 @@ async def publisher(
             server = aiohttp.web.Server(self.handle_request)
             runner = aiohttp.web.ServerRunner(server)
             await runner.setup()
-            site = aiohttp.web.UnixSite(runner, self.pub_path, ssl_context=ctx)
-            log.info("Publisher binding to socket %s", self.pub_path)
+            with salt.utils.files.set_umask(0o177):
+                log.info("Publisher binding to socket %s", self.pub_path)
+                site = aiohttp.web.UnixSite(runner, self.pub_path, ssl_context=ctx)
+                await site.start()
+                os.chmod(self.pub_path, self.pub_path_perms)
         else:
             sock = _get_socket(self.opts)
             sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
@@ -352,14 +362,15 @@ async def publisher(
             await runner.setup()
             site = aiohttp.web.SockSite(runner, sock, ssl_context=ctx)
             log.info("Publisher binding to socket %s:%s", self.pub_host, self.pub_port)
-        await site.start()
+            await site.start()
 
         self._pub_payload = publish_payload
         if self.pull_path:
             with salt.utils.files.set_umask(0o177):
                 self.puller = await asyncio.start_unix_server(
                     self.pull_handler, self.pull_path
                 )
+                os.chmod(self.pull_path, self.pull_path_perms)
         else:
             self.puller = await asyncio.start_server(
                 self.pull_handler, self.pull_host, self.pull_port

salt/transport/zeromq.py

@@ -852,6 +852,8 @@ def __init__(
         pull_host=None,
         pull_port=None,
         pull_path=None,
+        pull_path_perms=0o600,
+        pub_path_perms=0o600,
     ):
         self.opts = opts
         self.pub_host = pub_host
@@ -864,6 +866,8 @@ def __init__(
         self.pull_host = pull_host
         self.pull_port = pull_port
         self.pull_path = pull_path
+        self.pub_path_perms = pub_path_perms
+        self.pull_path_perms = pull_path_perms
         if pull_path:
             self.pull_uri = f"ipc://{pull_path}"
         else:
@@ -930,14 +934,14 @@ def _get_sockets(self, context, ioloop):
             if self.pub_path:
                 os.chmod(  # nosec
                     self.pub_path,
-                    0o600,
+                    self.pub_path_perms,
                 )
             log.info("Starting the Salt Puller on %s", self.pull_uri)
             pull_sock.bind(self.pull_uri)
             if self.pull_path:
                 os.chmod(  # nosec
                     self.pull_path,
-                    0o600,
+                    self.pull_path_perms,
                 )
         return pull_sock, pub_sock, monitor
 

tests/pytests/functional/states/test_pkg.py

@@ -231,10 +231,13 @@ def test_pkg_002_installed_with_version(PKG_TARGETS, states, latest_version):
 
 @pytest.mark.requires_salt_states("pkg.installed", "pkg.removed")
 @pytest.mark.slow_test
-def test_pkg_003_installed_multipkg(caplog, PKG_TARGETS, modules, states):
+def test_pkg_003_installed_multipkg(caplog, PKG_TARGETS, modules, states, grains):
     """
     This is a destructive test as it installs and then removes two packages
     """
+    if grains["os_family"] == "Arch":
+        pytest.skip("Arch needs refresh_db logic added to golden image")
+
     version = modules.pkg.version(*PKG_TARGETS)
 
     # If this assert fails, we need to find new targets, this test needs to
@@ -256,10 +259,14 @@ def test_pkg_003_installed_multipkg(caplog, PKG_TARGETS, modules, states):
 @pytest.mark.usefixtures("VERSION_SPEC_SUPPORTED")
 @pytest.mark.requires_salt_states("pkg.installed", "pkg.removed")
 @pytest.mark.slow_test
-def test_pkg_004_installed_multipkg_with_version(PKG_TARGETS, latest_version, states):
+def test_pkg_004_installed_multipkg_with_version(
+    PKG_TARGETS, latest_version, states, grains
+):
     """
     This is a destructive test as it installs and then removes two packages
     """
+    if grains["os_family"] == "Arch":
+        pytest.skip("Arch needs refresh_db logic added to golden image")
     version = latest_version(PKG_TARGETS[0])
 
     # If this assert fails, we need to find new targets, this test needs to
@@ -868,6 +875,7 @@ def test_pkg_cap_003_installed_multipkg_with_version(
     latest_version,
     modules,
     states,
+    grains,
 ):
     """
     This is a destructive test as it installs and then removes two packages