Syndics honor MoM acl

changelog/62618.fixed

@@ -0,0 +1 @@
+Fixed syndic eauth. Now jobs will be published when a valid eauth user is targeting allowed minions/functions.

changelog/62933.fixed

@@ -0,0 +1 @@
+Restored channel for Syndic minions to send job returns to the Salt master.

doc/topics/hardening.rst

@@ -150,3 +150,13 @@ Run the following on the Salt minion:
 
 .. _salt-users: https://groups.google.com/forum/#!forum/salt-users
 .. _salt-announce: https://groups.google.com/forum/#!forum/salt-announce
+
+
+Hardening of syndic setups
+==========================
+
+Syndics must be run as the same user as their syndic master process. The master
+of master's will include publisher ACL information in jobs sent to downstream
+masters via syndics. This means that any minions connected directly to a master
+of masters will also receive ACL information in jobs being published. For the
+most secure setup, only connect syndics directly to master of masters.

doc/topics/topology/syndic.rst

@@ -21,14 +21,6 @@ node and the local ``salt-master`` daemon.  This gives the Master node control
 over the Minion nodes attached to the ``salt-master`` daemon running on the
 Syndic node.
 
-.. warning::
-
-    Salt does not officially support Syndic and :ref:`external auth or
-    publisher_acl<acl-eauth>`. It's possible that it might work under certain
-    circumstances, but comprehensive support is lacking. See `issue #62618 on
-    GitHub <https://github.com/saltstack/salt/issues/62618>`_ for more
-    information. Currently Syndic is only expected to work when running Salt as
-    root, though work is scheduled to fix this in Salt 3006 (Sulfur).
 
 Configuring the Syndic
 ======================
@@ -71,6 +63,10 @@ The :conf_master:`order_masters` option configures the Master node to send
 extra information with its publications that is needed by Syndic nodes
 connected directly to it.
 
+.. warning::
+   The syndic process must be run as the same user as the syndic master.
+
+
 .. note::
 
     Each Syndic must provide its own ``file_roots`` directory. Files will not

salt/client/__init__.py

@@ -823,7 +823,6 @@ def cmd_cli(
                 listen=True,
                 **kwargs
             )
-
             if not self.pub_data:
                 yield self.pub_data
             else:
@@ -1191,6 +1190,13 @@ def get_iter_returns(
                 # if we got None, then there were no events
                 if raw is None:
                     break
+                if "error" in raw.get("data", {}):
+                    yield {
+                        "error": {
+                            "name": "AuthorizationError",
+                            "message": "Authorization error occurred.",
+                        }
+                    }
                 if "minions" in raw.get("data", {}):
                     minions.update(raw["data"]["minions"])
                     if "missing" in raw.get("data", {}):
@@ -1707,6 +1713,8 @@ def get_cli_event_returns(
                                     "retcode": salt.defaults.exitcodes.EX_GENERIC,
                                 }
                             }
+                elif "error" in min_ret:
+                    raise AuthorizationError("Authorization error occurred")
                 else:
                     yield {id_: min_ret}
 
@@ -1825,11 +1833,7 @@ def _prep_pub(self, tgt, fun, arg, tgt_type, ret, jid, timeout, **kwargs):
         if kwargs:
             payload_kwargs["kwargs"] = kwargs
 
-        # If we have a salt user, add it to the payload
-        if self.opts["syndic_master"] and "user" in kwargs:
-            payload_kwargs["user"] = kwargs["user"]
-        elif self.salt_user:
-            payload_kwargs["user"] = self.salt_user
+        payload_kwargs["user"] = self.salt_user
 
         # If we're a syndication master, pass the timeout
         if self.opts["order_masters"]:

salt/master.py

@@ -2167,14 +2167,17 @@ def publish(self, clear_load):
             }
 
         # Retrieve the minions list
-        delimiter = clear_load.get("kwargs", {}).get("delimiter", DEFAULT_TARGET_DELIM)
+        delimiter = extra.get("delimiter", DEFAULT_TARGET_DELIM)
+
         _res = self.ckminions.check_minions(
             clear_load["tgt"], clear_load.get("tgt_type", "glob"), delimiter
         )
         minions = _res.get("minions", list())
         missing = _res.get("missing", list())
         ssh_minions = _res.get("ssh_minions", False)
 
+        auth_key = clear_load.get("key", None)
+
         # Check for external auth calls and authenticate
         auth_type, err_name, key, sensitive_load_keys = self._prep_auth_info(extra)
         if auth_type == "user":
@@ -2184,20 +2187,36 @@ def publish(self, clear_load):
         else:
             auth_check = self.loadauth.check_authentication(extra, auth_type)
 
-        # Setup authorization list variable and error information
-        auth_list = auth_check.get("auth_list", [])
+        # Setup authorization list
+        syndic_auth_list = None
+        if "auth_list" in extra:
+            syndic_auth_list = extra.pop("auth_list", [])
+        # An auth_list was provided by the syndic and we're running as the same
+        # user as the salt master process.
+        if (
+            syndic_auth_list is not None
+            and auth_key == key[self.opts.get("user", "root")]
+        ):
+            auth_list = syndic_auth_list
+        else:
+            auth_list = auth_check.get("auth_list", [])
+
         err_msg = 'Authentication failure of type "{}" occurred.'.format(auth_type)
 
         if auth_check.get("error"):
             # Authentication error occurred: do not continue.
             log.warning(err_msg)
-            return {
+            err = {
                 "error": {
                     "name": "AuthenticationError",
                     "message": "Authentication error occurred.",
                 }
             }
-
+            if "jid" in clear_load:
+                self.event.fire_event(
+                    {**clear_load, **err}, tagify([clear_load["jid"], "error"], "job")
+                )
+            return err
         # All Token, Eauth, and non-root users must pass the authorization check
         if auth_type != "user" or (auth_type == "user" and auth_list):
             # Authorize the request
@@ -2226,12 +2245,18 @@ def publish(self, clear_load):
                         extra["username"],
                     )
                 log.warning(err_msg)
-                return {
+                err = {
                     "error": {
                         "name": "AuthorizationError",
                         "message": "Authorization error occurred.",
                     }
                 }
+                if "jid" in clear_load:
+                    self.event.fire_event(
+                        {**clear_load, **err},
+                        tagify([clear_load["jid"], "error"], "job"),
+                    )
+                return err
 
             # Perform some specific auth_type tasks after the authorization check
             if auth_type == "token":
@@ -2264,6 +2289,9 @@ def publish(self, clear_load):
             return {"enc": "clear", "load": {"error": "Master failed to assign jid"}}
         payload = self._prep_pub(minions, jid, clear_load, extra, missing)
 
+        if self.opts.get("order_masters"):
+            payload["auth_list"] = auth_list
+
         # Send it!
         self._send_ssh_pub(payload, ssh_minions=ssh_minions)
         self._send_pub(payload)

salt/minion.py

@@ -3278,7 +3278,8 @@ def syndic_cmd(self, data):
         # Set up default tgt_type
         if "tgt_type" not in data:
             data["tgt_type"] = "glob"
-        kwargs = {}
+
+        kwargs = {"auth_list": data.pop("auth_list", [])}
 
         # optionally add a few fields to the publish data
         for field in (
@@ -3306,6 +3307,32 @@ def timeout_handler(*args):
                 **kwargs
             )
 
+    def _send_req_sync(self, load, timeout):
+        if self.opts["minion_sign_messages"]:
+            log.trace("Signing event to be published onto the bus.")
+            minion_privkey_path = os.path.join(self.opts["pki_dir"], "minion.pem")
+            sig = salt.crypt.sign_message(
+                minion_privkey_path, salt.serializers.msgpack.serialize(load)
+            )
+            load["sig"] = sig
+        return self.req_channel.send(
+            load, timeout=timeout, tries=self.opts["return_retry_tries"]
+        )
+
+    @salt.ext.tornado.gen.coroutine
+    def _send_req_async(self, load, timeout):
+        if self.opts["minion_sign_messages"]:
+            log.trace("Signing event to be published onto the bus.")
+            minion_privkey_path = os.path.join(self.opts["pki_dir"], "minion.pem")
+            sig = salt.crypt.sign_message(
+                minion_privkey_path, salt.serializers.msgpack.serialize(load)
+            )
+            load["sig"] = sig
+        ret = yield self.async_req_channel.send(
+            load, timeout=timeout, tries=self.opts["return_retry_tries"]
+        )
+        return ret
+
     def fire_master_syndic_start(self):
         # Send an event to the master that the minion is live
         if self.opts["enable_legacy_startup_events"]:
@@ -3335,6 +3362,8 @@ def tune_in_no_block(self):
 
         # add handler to subscriber
         self.pub_channel.on_recv(self._process_cmd_socket)
+        self.req_channel = salt.channel.client.ReqChannel.factory(self.opts)
+        self.async_req_channel = salt.channel.client.ReqChannel.factory(self.opts)
 
     def _process_cmd_socket(self, payload):
         if payload is not None and payload["enc"] == "aes":

salt/utils/minions.py

@@ -748,16 +748,15 @@ def validate_tgt(self, valid, expr, tgt_type, minions=None, expr_form=None):
         """
 
         v_minions = set(self.check_minions(valid, "compound").get("minions", []))
-        if not v_minions:
-            # There are no valid minions, so it doesn't matter what we are
-            # targeting - this is a fail.
-            return False
         if minions is None:
             _res = self.check_minions(expr, tgt_type)
             minions = set(_res["minions"])
         else:
             minions = set(minions)
-        return minions.issubset(v_minions)
+        d_bool = not bool(minions.difference(v_minions))
+        if len(v_minions) == len(minions) and d_bool:
+            return True
+        return d_bool
 
     def match_check(self, regex, fun):
         """