Several fixes for security issues

* Several fixes for security issues

(bsc#1244561, CVE-2024-38822)
(bsc#1244564, CVE-2024-38823)
(bsc#1244565, CVE-2024-38824)
(bsc#1244566, CVE-2024-38825)
(bsc#1244567, CVE-2025-22240)
(bsc#1244568, CVE-2025-22236)
(bsc#1244570, CVE-2025-22241)
(bsc#1244571, CVE-2025-22237)
(bsc#1244572, CVE-2025-22238)
(bsc#1244574, CVE-2025-22239)
(bsc#1244575, CVE-2025-22242)

Request server hardening
- Each minion get's it's own aes session for request server
  communication.
- Request client always includes id and token, these are always
  validated server side.
- Add timestamp and enforce configurable ttl for request server
  messages.

Other relevant commit messages:

- Add deprecation message to salt.auth.pki
- Add test and fix for file_recv cve
- Prevent traversal in local_cache::save_minions
- Fix traversal in gitfs find_file
- Fix traversals in salt.utils.virt
- Fix traversal in pub_ret
- On-demand pillar fix
- Include url validation tests
- Minion event filtering
- Reasonable failures when pillars timeout
- Adjust and fix code and tests after backporting
  to openSUSE/release/3006.0

Co-authored-by: Pablo Suárez Hernández <[email protected]>

* Fix test_pillar_timeout unit test in Salt Shaker

* Fix tests failures on functional/channel/test_req_channel.py

* Fix gitfs test failures due uncomplete cleanup

* Fix cp.push module function and its integration test (#68053)

fix file_recv path verification for subdirs

Adapt backport to fit openSUSE/release/3006.0

* Make send_req_async wait longer (#68085)

Allow send_req_async to wait longer when sending a return back to the
master. The minion should wait at least as long as the max possible
return timeout.

Update creds when session key changes

Add unit test to validate session key rotation

Add changelog for #68079

* Remove token to prevent decoding errors (#68084)

Clean up verify_load calls in master request server

Remove tok in salt.channel.ReqServer.validate_token so it is not passed
to the request handlers.

Add tests around payload token removal

Add changelog for #68076

* Fix checking of non-url style git remotes (#68089)

Handle [email protected]/.. style remotes

Fix checking of non-url style git remotes

Fixes handling of git@hostname:/path/repo style remotes. Takes initial
version from #68082 and fixes it. Still uses the shortcut of converting
the remote to ssh:// URL style.

Split out converting remote to URL

Splits out converting remotes to URL form to allow testing of that
conversion - without doing that risk issues with the regex

Add additional test cases

Add additional test cases based on gitfs docs and what they say should
be valid

Make utility functions classmethods

Fix key vs remote wart

Allow subdirs in GitFS find_file check (#68083)

Add test for find_file in sub directories

Add changelog for #68072

---------

Co-authored-by: Daniel A. Wozniak <[email protected]>
Co-authored-by: hurzhurz <[email protected]>

Author: 3 people

changelog/67941.fixed.md

@@ -0,0 +1 @@
+Fix cp.push module function and its integration test

changelog/68033.fixed.md

@@ -0,0 +1,56 @@
+CVE-2024-38822
+Multiple methods in the salt master skip minion token validation. Therefore a misbehaving minion can impersonate another minion.
+
+CVSS 2.7 V:N/AC:L/PR:H/UI:N/S:U/C:N/I:L/A:N
+
+CVE-2024-38823
+Salt's request server is vulnerable to replay attacks when not using a TLS encrypted transport.
+
+CVSS Score 2.7 AV:N/AC:L/PR:H/UI:N/S:U/C:N/I:L/A:N
+
+CVE-2024-38824
+Directory traversal vulnerability in recv_file method allows arbitrary files to be written to the master cache directory.
+
+CVSS Score 9.6 AV:N/AC:L/PR:L/UI:N/S:C/C:H/I:H/A:N
+
+CVE-2024-38825
+The salt.auth.pki module does not properly authenticate callers. The "password" field contains a public certificate which is validated against a CA certificate by the module. This is not pki authentication, as the caller does not need access to the corresponding private key for the authentication attempt to be accepted.
+
+CVSS Score 6.4 AV:N/AC:L/PR:L/UI:N/S:C/C:L/I:L/A:N
+
+CVE-2025-22236
+Minion event bus authorization bypass. An attacker with access to a minion key can craft a message which may be able to execute a job on other minions (>= 3007.0).
+
+CVSS 8.1 AV:L/AC:L/PR:H/UI:N/S:C/C:H/I:H/A:L
+
+CVE-2025-22237
+An attacker with access to a minion key can exploit the 'on demand' pillar functionality with a specially crafted git url which could cause and arbitrary command to be run on the master with the same privileges as the master process.
+
+CVSS 6.7 AV:L/AC:L/PR:H/UI:N/S:U/C:H/I:H/A:H
+
+CVE-2025-22238
+Directory traversal attack in minion file cache creation. The master's default cache is vulnerable to a directory traversal attack. Which could be leveraged to write or overwrite 'cache' files outside of the cache directory.
+
+CVSS 4.2 AV:L/AC:L/PR:H/UI:R/S:U/C:N/I:H/A:N
+
+CVE-2025-22239
+Arbitrary event injection on Salt Master. The master's "_minion_event" method can be used by and authorized minion to send arbitrary events onto the master's event bus.
+
+CVSS 8.1 AV:L/AC:L/PR:H/UI:N/S:C/C:H/I:H/A:L
+
+CVE-2025-22240
+Arbitrary directory creation or file deletion. In the find_file method of the GitFS class, a path is created using os.path.join using unvalidated input from the “tgt_env” variable. This can be exploited by an attacker to delete any file on the Master's process has permissions to
+
+CVSS 6.3 AV:L/AC:H/PR:H/UI:R/S:U/C:H/I:H/A:H
+
+CVE-2025-22241
+File contents overwrite the VirtKey class is called when “on-demand pillar” data is requested and uses un-validated input to create paths to the “pki directory”. The functionality is used to auto-accept Minion authentication keys based on a pre-placed “authorization file” at a specific location and is present in the default configuration.
+
+CVSS 5.6 AV:L/AC:H/PR:H/UI:R/S:U/C:H/I:H/A:N
+
+CVE-2025-22242
+Worker process denial of service through file read operation. .A vulnerability exists in the Master's “pub_ret” method which is exposed to all minions. The un-sanitized input value “jid” is used to construct a path which is then opened for reading. An attacker could exploit this vulnerabilities by attempting to read from a filename that will not return any data, e.g. by targeting a pipe node on the proc file system.
+
+CVSS 5.6 AV:L/AC:H/PR:H/UI:R/S:U/C:H/I:N/A:H
+
+This release also includes sqlite 3.50.1 to address CVE-2025-29087

changelog/68072.fixed.md

@@ -0,0 +1 @@
+Fix GitFS file_find for file in sub-directories

changelog/68076.fixed.md

@@ -0,0 +1 @@
+Token validation removes token from request handler payload

changelog/68079.fixed.md

@@ -0,0 +1 @@
+Fix minion connectivity issues by ensuring auth notices refreshed session token

changelog/68087.fixed.md

@@ -0,0 +1 @@
+Fix file_recv path verification to allow subdirs used by cp.push

salt/auth/pki.py

@@ -17,6 +17,7 @@
 import logging
 
 import salt.utils.files
+import salt.utils.versions
 
 # pylint: disable=import-error
 try:
@@ -30,7 +31,7 @@
             from Cryptodome.Util import asn1
         except ImportError:
             from Crypto.Util import asn1  # nosec
-        import OpenSSL
+        import OpenSSL  # pylint: disable=W8410
     HAS_DEPS = True
 except ImportError:
     HAS_DEPS = False
@@ -71,6 +72,10 @@ def auth(username, password, **kwargs):
             your_user:
               - .*
     """
+    salt.utils.versions.warn_until(
+        "Argon",
+        "This module has been deprecated as it is known to be insecure.",
+    )
     pem = password
     cacert_file = __salt__["config.get"]("external_auth:pki:ca_file")
 

salt/channel/client.py

@@ -40,6 +40,9 @@
 
 log = logging.getLogger(__name__)
 
+REQUEST_CHANNEL_TIMEOUT = 60
+REQUEST_CHANNEL_TRIES = 3
+
 
 class ReqChannel:
     """
@@ -120,6 +123,9 @@ def factory(cls, opts, **kwargs):
         if io_loop is None:
             io_loop = salt.ext.tornado.ioloop.IOLoop.current()
 
+        timeout = opts.get("request_channel_timeout", REQUEST_CHANNEL_TIMEOUT)
+        tries = opts.get("request_channel_tries", REQUEST_CHANNEL_TRIES)
+
         crypt = kwargs.get("crypt", "aes")
         if crypt != "clear":
             # we don't need to worry about auth as a kwarg, since its a singleton
@@ -128,16 +134,26 @@ def factory(cls, opts, **kwargs):
             auth = None
 
         transport = salt.transport.request_client(opts, io_loop)
-        return cls(opts, transport, auth)
+        return cls(opts, transport, auth, tries=tries, timeout=timeout)
 
-    def __init__(self, opts, transport, auth, **kwargs):
+    def __init__(
+        self,
+        opts,
+        transport,
+        auth,
+        timeout=REQUEST_CHANNEL_TIMEOUT,
+        tries=REQUEST_CHANNEL_TRIES,
+        **kwargs
+    ):
         self.opts = dict(opts)
         self.transport = transport
         self.auth = auth
         self.master_pubkey_path = None
         if self.auth:
             self.master_pubkey_path = os.path.join(self.opts["pki_dir"], self.auth.mpub)
         self._closing = False
+        self.timeout = timeout
+        self.tries = tries
 
     @property
     def crypt(self):
@@ -149,35 +165,90 @@ def crypt(self):
     def ttype(self):
         return self.transport.ttype
 
-    def _package_load(self, load):
-        return {
+    def _package_load(self, load, nonce=None):
+        """
+        Prepare the load to be sent over the wire.
+
+        For aes channels add a nonce, timestamp and signed token to the load
+        before encrypting it using our aes session key. Then wrap the encrypted
+        load with some meta data. For 'clear' encryption, no extra feilds are
+        added to the load. The unencyrpted load is wrapped with meta data.
+        """
+        if self.crypt == "aes":
+            if nonce is None:
+                nonce = uuid.uuid4().hex
+            try:
+                load["nonce"] = nonce
+                load["ts"] = int(time.time())
+                load["tok"] = self.auth.gen_token(b"salt")
+                load["id"] = self.opts["id"]
+            except TypeError:
+                # Backwards compatability for non dict loads, let the load get
+                # sent and fail to authenticate.
+                log.warning(
+                    "Invalid load passed to request channel. Type is %s should be dict.",
+                    type(load),
+                )
+
+            load = self.auth.session_crypticle.dumps(load)
+
+        ret = {
             "enc": self.crypt,
             "load": load,
-            "version": 2,
+            "version": 3,
         }
+        if self.crypt == "aes":
+            ret["id"] = self.opts["id"]
+        return ret
+
+    @salt.ext.tornado.gen.coroutine
+    def _send_with_retry(self, load, tries, timeout):
+        _try = 1
+        while True:
+            try:
+                ret = yield self.transport.send(
+                    load,
+                    timeout=timeout,
+                )
+                break
+            except Exception as exc:  # pylint: disable=broad-except
+                log.trace("Failed to send msg %r", exc)
+                if _try >= tries:
+                    raise
+                else:
+                    _try += 1
+                    continue
+        raise salt.ext.tornado.gen.Return(ret)
 
     @salt.ext.tornado.gen.coroutine
     def crypted_transfer_decode_dictentry(
         self,
         load,
         dictkey=None,
-        timeout=60,
+        timeout=None,
+        tries=None,
     ):
-        nonce = uuid.uuid4().hex
-        load["nonce"] = nonce
+        if timeout is None:
+            timeout = self.timeout
+        if tries is None:
+            tries = self.tries
         if not self.auth.authenticated:
             yield self.auth.authenticate()
-        ret = yield self.transport.send(
-            self._package_load(self.auth.crypticle.dumps(load)),
-            timeout=timeout,
+
+        nonce = uuid.uuid4().hex
+        ret = yield self._send_with_retry(
+            self._package_load(load, nonce),
+            tries,
+            timeout,
         )
         key = self.auth.get_keys()
         if "key" not in ret:
             # Reauth in the case our key is deleted on the master side.
             yield self.auth.authenticate()
-            ret = yield self.transport.send(
-                self._package_load(self.auth.crypticle.dumps(load)),
-                timeout=timeout,
+            ret = yield self._send_with_retry(
+                self._package_load(load, nonce),
+                tries,
+                timeout,
             )
         if HAS_M2:
             aes = key.private_decrypt(ret["key"], RSA.pkcs1_oaep_padding)
@@ -209,7 +280,7 @@ def verify_signature(self, data, sig):
         return salt.crypt.verify_signature(self.master_pubkey_path, data, sig)
 
     @salt.ext.tornado.gen.coroutine
-    def _crypted_transfer(self, load, timeout=60, raw=False):
+    def _crypted_transfer(self, load, timeout, raw=False):
         """
         Send a load across the wire, with encryption
 
@@ -222,25 +293,24 @@ def _crypted_transfer(self, load, timeout=60, raw=False):
         :param dict load: A load to send across the wire
         :param int timeout: The number of seconds on a response before failing
         """
-        nonce = uuid.uuid4().hex
-        if load and isinstance(load, dict):
-            load["nonce"] = nonce
 
         @salt.ext.tornado.gen.coroutine
         def _do_transfer():
             # Yield control to the caller. When send() completes, resume by populating data with the Future.result
+            nonce = uuid.uuid4().hex
             data = yield self.transport.send(
-                self._package_load(self.auth.crypticle.dumps(load)),
+                self._package_load(load, nonce),
                 timeout=timeout,
             )
             # we may not have always data
             # as for example for saltcall ret submission, this is a blind
             # communication, we do not subscribe to return events, we just
             # upload the results to the master
             if data:
-                data = self.auth.crypticle.loads(data, raw, nonce=nonce)
+                data = self.auth.session_crypticle.loads(data, raw, nonce=nonce)
             if not raw or self.ttype == "tcp":  # XXX Why is this needed for tcp
                 data = salt.transport.frame.decode_embedded_strs(data)
+
             raise salt.ext.tornado.gen.Return(data)
 
         if not self.auth.authenticated:
@@ -256,7 +326,7 @@ def _do_transfer():
         raise salt.ext.tornado.gen.Return(ret)
 
     @salt.ext.tornado.gen.coroutine
-    def _uncrypted_transfer(self, load, timeout=60):
+    def _uncrypted_transfer(self, load, timeout):
         """
         Send a load across the wire in cleartext
 
@@ -271,14 +341,18 @@ def _uncrypted_transfer(self, load, timeout=60):
         raise salt.ext.tornado.gen.Return(ret)
 
     @salt.ext.tornado.gen.coroutine
-    def send(self, load, tries=3, timeout=60, raw=False):
+    def send(self, load, tries=None, timeout=None, raw=False):
         """
         Send a request, return a future which will complete when we send the message
 
         :param dict load: A load to send across the wire
         :param int tries: The number of times to make before failure
         :param int timeout: The number of seconds on a response before failing
         """
+        if timeout is None:
+            timeout = self.timeout
+        if tries is None:
+            tries = self.tries
         _try = 1
         while True:
             try:
@@ -427,7 +501,7 @@ def _package_load(self, load):
         return {
             "enc": self.crypt,
             "load": load,
-            "version": 2,
+            "version": 3,
         }
 
     @salt.ext.tornado.gen.coroutine