From 4093dfc9942ef816266ec818fdf4a0061df411c15e8c954c20192d4af8544a82 Mon Sep 17 00:00:00 2001 From: Victor Zhestkov Date: Tue, 22 Aug 2023 12:20:45 +0000 Subject: [PATCH] Accepting request 1105250 from home:PSuarezHernandez:branches:systemsmanagement:saltstack - Make sure configured user is properly set by Salt (bsc#1210994) - Do not fail on bad message pack message (bsc#1213441, CVE-2023-20897) - Fix broken tests to make them running in the testsuite - Prevent possible exceptions on salt.utils.user.get_group_dict (bsc#1212794) - Added: * do-not-fail-on-bad-message-pack-message-bsc-1213441-.patch * fix-tests-to-make-them-running-with-salt-testsuite.patch * prevent-possible-exceptions-on-salt.utils.user.get_g.patch * make-sure-configured-user-is-properly-set-by-salt-bs.patch OBS-URL: https://build.opensuse.org/request/show/1105250 OBS-URL: https://build.opensuse.org/package/show/systemsmanagement:saltstack/salt?expand=0&rev=213 --- _lastrevision | 2 +- ...ad-message-pack-message-bsc-1213441-.patch | 155 ++++ ...ake-them-running-with-salt-testsuite.patch | 841 ++++++++++++++++++ ...ured-user-is-properly-set-by-salt-bs.patch | 204 +++++ ...-exceptions-on-salt.utils.user.get_g.patch | 68 ++ salt.changes | 14 + salt.spec | 9 + 7 files changed, 1292 insertions(+), 1 deletion(-) create mode 100644 do-not-fail-on-bad-message-pack-message-bsc-1213441-.patch create mode 100644 fix-tests-to-make-them-running-with-salt-testsuite.patch create mode 100644 make-sure-configured-user-is-properly-set-by-salt-bs.patch create mode 100644 prevent-possible-exceptions-on-salt.utils.user.get_g.patch diff --git a/_lastrevision b/_lastrevision index 282ec6c..bc18ba2 100644 --- a/_lastrevision +++ b/_lastrevision @@ -1 +1 @@ -26c7f283aef34794887afd5a4199be1018c5c592 \ No newline at end of file +9c59105c906c6b5eb9345c79e71e7d33f119ed22 \ No newline at end of file diff --git a/do-not-fail-on-bad-message-pack-message-bsc-1213441-.patch b/do-not-fail-on-bad-message-pack-message-bsc-1213441-.patch new file mode 100644 index 0000000..8a02fff --- /dev/null +++ b/do-not-fail-on-bad-message-pack-message-bsc-1213441-.patch @@ -0,0 +1,155 @@ +From da544d7ab09899717e57a02321928ceaf3c6465c Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?= + +Date: Tue, 22 Aug 2023 11:43:46 +0100 +Subject: [PATCH] Do not fail on bad message pack message (bsc#1213441, + CVE-2023-20897) (#595) + +* Do not fail on bad message pack message + +Fix unit test after backporting to openSUSE/release/3006.0 + +* Better error message when inconsistent decoded payload + +--------- + +Co-authored-by: Daniel A. Wozniak +--- + salt/channel/server.py | 10 +++ + salt/transport/zeromq.py | 6 +- + tests/pytests/unit/transport/test_zeromq.py | 69 +++++++++++++++++++++ + 3 files changed, 84 insertions(+), 1 deletion(-) + +diff --git a/salt/channel/server.py b/salt/channel/server.py +index a2117f2934..b6d51fef08 100644 +--- a/salt/channel/server.py ++++ b/salt/channel/server.py +@@ -22,6 +22,7 @@ import salt.utils.minions + import salt.utils.platform + import salt.utils.stringutils + import salt.utils.verify ++from salt.exceptions import SaltDeserializationError + from salt.utils.cache import CacheCli + + try: +@@ -252,6 +253,15 @@ class ReqServerChannel: + return False + + def _decode_payload(self, payload): ++ # Sometimes msgpack deserialization of random bytes could be successful, ++ # so we need to ensure payload in good shape to process this function. ++ if ( ++ not isinstance(payload, dict) ++ or "enc" not in payload ++ or "load" not in payload ++ ): ++ raise SaltDeserializationError("bad load received on socket!") ++ + # we need to decrypt it + if payload["enc"] == "aes": + try: +diff --git a/salt/transport/zeromq.py b/salt/transport/zeromq.py +index 3ec7f7726c..7cc6b9987f 100644 +--- a/salt/transport/zeromq.py ++++ b/salt/transport/zeromq.py +@@ -428,7 +428,11 @@ class RequestServer(salt.transport.base.DaemonizedRequestServer): + + @salt.ext.tornado.gen.coroutine + def handle_message(self, stream, payload): +- payload = self.decode_payload(payload) ++ try: ++ payload = self.decode_payload(payload) ++ except salt.exceptions.SaltDeserializationError: ++ self.stream.send(self.encode_payload({"msg": "bad load"})) ++ return + # XXX: Is header really needed? + reply = yield self.message_handler(payload) + self.stream.send(self.encode_payload(reply)) +diff --git a/tests/pytests/unit/transport/test_zeromq.py b/tests/pytests/unit/transport/test_zeromq.py +index 10bb4917b8..c7cbc53864 100644 +--- a/tests/pytests/unit/transport/test_zeromq.py ++++ b/tests/pytests/unit/transport/test_zeromq.py +@@ -11,6 +11,7 @@ import threading + import time + import uuid + ++import msgpack + import pytest + + import salt.channel.client +@@ -1404,3 +1405,71 @@ async def test_req_chan_auth_v2_new_minion_without_master_pub(pki_dir, io_loop): + assert "sig" in ret + ret = client.auth.handle_signin_response(signin_payload, ret) + assert ret == "retry" ++ ++ ++async def test_req_server_garbage_request(io_loop): ++ """ ++ Validate invalid msgpack messages will not raise exceptions in the ++ RequestServers's message handler. ++ """ ++ opts = salt.config.master_config("") ++ request_server = salt.transport.zeromq.RequestServer(opts) ++ ++ def message_handler(payload): ++ return payload ++ ++ request_server.post_fork(message_handler, io_loop) ++ ++ byts = msgpack.dumps({"foo": "bar"}) ++ badbyts = byts[:3] + b"^M" + byts[3:] ++ ++ valid_response = msgpack.dumps({"msg": "bad load"}) ++ ++ with MagicMock() as stream: ++ request_server.stream = stream ++ ++ try: ++ await request_server.handle_message(stream, badbyts) ++ except Exception as exc: # pylint: disable=broad-except ++ pytest.fail("Exception was raised {}".format(exc)) ++ ++ request_server.stream.send.assert_called_once_with(valid_response) ++ ++ ++async def test_req_chan_bad_payload_to_decode(pki_dir, io_loop): ++ opts = { ++ "master_uri": "tcp://127.0.0.1:4506", ++ "interface": "127.0.0.1", ++ "ret_port": 4506, ++ "ipv6": False, ++ "sock_dir": ".", ++ "pki_dir": str(pki_dir.joinpath("minion")), ++ "id": "minion", ++ "__role": "minion", ++ "keysize": 4096, ++ "max_minions": 0, ++ "auto_accept": False, ++ "open_mode": False, ++ "key_pass": None, ++ "publish_port": 4505, ++ "auth_mode": 1, ++ "acceptance_wait_time": 3, ++ "acceptance_wait_time_max": 3, ++ } ++ SMaster.secrets["aes"] = { ++ "secret": multiprocessing.Array( ++ ctypes.c_char, ++ salt.utils.stringutils.to_bytes(salt.crypt.Crypticle.generate_key_string()), ++ ), ++ "reload": salt.crypt.Crypticle.generate_key_string, ++ } ++ master_opts = dict(opts, pki_dir=str(pki_dir.joinpath("master"))) ++ master_opts["master_sign_pubkey"] = False ++ server = salt.channel.server.ReqServerChannel.factory(master_opts) ++ ++ with pytest.raises(salt.exceptions.SaltDeserializationError): ++ server._decode_payload(None) ++ with pytest.raises(salt.exceptions.SaltDeserializationError): ++ server._decode_payload({}) ++ with pytest.raises(salt.exceptions.SaltDeserializationError): ++ server._decode_payload(12345) +-- +2.41.0 + + diff --git a/fix-tests-to-make-them-running-with-salt-testsuite.patch b/fix-tests-to-make-them-running-with-salt-testsuite.patch new file mode 100644 index 0000000..42b9e6f --- /dev/null +++ b/fix-tests-to-make-them-running-with-salt-testsuite.patch @@ -0,0 +1,841 @@ +From 290d092c06dc378647dd1e49f000f012a7c07904 Mon Sep 17 00:00:00 2001 +From: vzhestkov +Date: Wed, 2 Aug 2023 16:13:49 +0200 +Subject: [PATCH] Fix tests to make them running with salt-testsuite + +--- + tests/pytests/unit/cli/test_batch_async.py | 718 +++++++++++---------- + tests/unit/cli/test_support.py | 6 +- + tests/unit/modules/test_saltsupport.py | 4 +- + 3 files changed, 364 insertions(+), 364 deletions(-) + +diff --git a/tests/pytests/unit/cli/test_batch_async.py b/tests/pytests/unit/cli/test_batch_async.py +index c0b708de76..e0774ffff3 100644 +--- a/tests/pytests/unit/cli/test_batch_async.py ++++ b/tests/pytests/unit/cli/test_batch_async.py +@@ -1,386 +1,392 @@ ++import pytest ++ + import salt.ext.tornado + from salt.cli.batch_async import BatchAsync +-from salt.ext.tornado.testing import AsyncTestCase + from tests.support.mock import MagicMock, patch +-from tests.support.unit import TestCase, skipIf +- +- +-class AsyncBatchTestCase(AsyncTestCase, TestCase): +- def setUp(self): +- self.io_loop = self.get_new_ioloop() +- opts = { +- "batch": "1", +- "conf_file": {}, +- "tgt": "*", +- "timeout": 5, +- "gather_job_timeout": 5, +- "batch_presence_ping_timeout": 1, +- "transport": None, +- "sock_dir": "", +- } +- +- with patch("salt.client.get_local_client", MagicMock(return_value=MagicMock())): +- with patch( +- "salt.cli.batch_async.batch_get_opts", MagicMock(return_value=opts) +- ): +- self.batch = BatchAsync( +- opts, +- MagicMock(side_effect=["1234", "1235", "1236"]), +- { +- "tgt": "", +- "fun": "", +- "kwargs": {"batch": "", "batch_presence_ping_timeout": 1}, +- }, +- ) +- +- def test_ping_jid(self): +- self.assertEqual(self.batch.ping_jid, "1234") +- +- def test_batch_jid(self): +- self.assertEqual(self.batch.batch_jid, "1235") +- +- def test_find_job_jid(self): +- self.assertEqual(self.batch.find_job_jid, "1236") +- +- def test_batch_size(self): +- """ +- Tests passing batch value as a number +- """ +- self.batch.opts = {"batch": "2", "timeout": 5} +- self.batch.minions = {"foo", "bar"} +- self.batch.start_batch() +- self.assertEqual(self.batch.batch_size, 2) +- +- @salt.ext.tornado.testing.gen_test +- def test_batch_start_on_batch_presence_ping_timeout(self): +- self.batch.event = MagicMock() +- future = salt.ext.tornado.gen.Future() +- future.set_result({"minions": ["foo", "bar"]}) +- self.batch.local.run_job_async.return_value = future +- ret = self.batch.start() +- # assert start_batch is called later with batch_presence_ping_timeout as param +- self.assertEqual( +- self.batch.event.io_loop.spawn_callback.call_args[0], +- (self.batch.start_batch,), +- ) +- # assert test.ping called +- self.assertEqual( +- self.batch.local.run_job_async.call_args[0], ("*", "test.ping", [], "glob") +- ) +- # assert targeted_minions == all minions matched by tgt +- self.assertEqual(self.batch.targeted_minions, {"foo", "bar"}) +- +- @salt.ext.tornado.testing.gen_test +- def test_batch_start_on_gather_job_timeout(self): +- self.batch.event = MagicMock() +- future = salt.ext.tornado.gen.Future() +- future.set_result({"minions": ["foo", "bar"]}) +- self.batch.local.run_job_async.return_value = future +- self.batch.batch_presence_ping_timeout = None +- ret = self.batch.start() +- # assert start_batch is called later with gather_job_timeout as param +- self.assertEqual( +- self.batch.event.io_loop.spawn_callback.call_args[0], +- (self.batch.start_batch,), +- ) + +- def test_batch_fire_start_event(self): +- self.batch.minions = {"foo", "bar"} +- self.batch.opts = {"batch": "2", "timeout": 5} +- self.batch.event = MagicMock() +- self.batch.metadata = {"mykey": "myvalue"} +- self.batch.start_batch() +- self.assertEqual( +- self.batch.event.fire_event.call_args[0], +- ( ++ ++@pytest.fixture ++def batch(temp_salt_master): ++ opts = { ++ "batch": "1", ++ "conf_file": {}, ++ "tgt": "*", ++ "timeout": 5, ++ "gather_job_timeout": 5, ++ "batch_presence_ping_timeout": 1, ++ "transport": None, ++ "sock_dir": "", ++ } ++ ++ with patch("salt.client.get_local_client", MagicMock(return_value=MagicMock())): ++ with patch("salt.cli.batch_async.batch_get_opts", MagicMock(return_value=opts)): ++ batch = BatchAsync( ++ opts, ++ MagicMock(side_effect=["1234", "1235", "1236"]), + { +- "available_minions": {"foo", "bar"}, +- "down_minions": set(), +- "metadata": self.batch.metadata, ++ "tgt": "", ++ "fun": "", ++ "kwargs": {"batch": "", "batch_presence_ping_timeout": 1}, + }, +- "salt/batch/1235/start", +- ), +- ) ++ ) ++ yield batch + +- @salt.ext.tornado.testing.gen_test +- def test_start_batch_calls_next(self): +- self.batch.run_next = MagicMock(return_value=MagicMock()) +- self.batch.event = MagicMock() +- self.batch.start_batch() +- self.assertEqual(self.batch.initialized, True) +- self.assertEqual( +- self.batch.event.io_loop.spawn_callback.call_args[0], (self.batch.run_next,) +- ) + +- def test_batch_fire_done_event(self): +- self.batch.targeted_minions = {"foo", "baz", "bar"} +- self.batch.minions = {"foo", "bar"} +- self.batch.done_minions = {"foo"} +- self.batch.timedout_minions = {"bar"} +- self.batch.event = MagicMock() +- self.batch.metadata = {"mykey": "myvalue"} +- old_event = self.batch.event +- self.batch.end_batch() +- self.assertEqual( +- old_event.fire_event.call_args[0], +- ( +- { +- "available_minions": {"foo", "bar"}, +- "done_minions": self.batch.done_minions, +- "down_minions": {"baz"}, +- "timedout_minions": self.batch.timedout_minions, +- "metadata": self.batch.metadata, +- }, +- "salt/batch/1235/done", +- ), +- ) ++def test_ping_jid(batch): ++ assert batch.ping_jid == "1234" + +- def test_batch__del__(self): +- batch = BatchAsync(MagicMock(), MagicMock(), MagicMock()) +- event = MagicMock() +- batch.event = event +- batch.__del__() +- self.assertEqual(batch.local, None) +- self.assertEqual(batch.event, None) +- self.assertEqual(batch.ioloop, None) +- +- def test_batch_close_safe(self): +- batch = BatchAsync(MagicMock(), MagicMock(), MagicMock()) +- event = MagicMock() +- batch.event = event +- batch.patterns = { +- ("salt/job/1234/ret/*", "find_job_return"), +- ("salt/job/4321/ret/*", "find_job_return"), +- } +- batch.close_safe() +- self.assertEqual(batch.local, None) +- self.assertEqual(batch.event, None) +- self.assertEqual(batch.ioloop, None) +- self.assertEqual(len(event.unsubscribe.mock_calls), 2) +- self.assertEqual(len(event.remove_event_handler.mock_calls), 1) +- +- @salt.ext.tornado.testing.gen_test +- def test_batch_next(self): +- self.batch.event = MagicMock() +- self.batch.opts["fun"] = "my.fun" +- self.batch.opts["arg"] = [] +- self.batch._get_next = MagicMock(return_value={"foo", "bar"}) +- self.batch.batch_size = 2 +- future = salt.ext.tornado.gen.Future() +- future.set_result({"minions": ["foo", "bar"]}) +- self.batch.local.run_job_async.return_value = future +- self.batch.run_next() +- self.assertEqual( +- self.batch.local.run_job_async.call_args[0], +- ({"foo", "bar"}, "my.fun", [], "list"), +- ) +- self.assertEqual( +- self.batch.event.io_loop.spawn_callback.call_args[0], +- (self.batch.find_job, {"foo", "bar"}), +- ) +- self.assertEqual(self.batch.active, {"bar", "foo"}) +- +- def test_next_batch(self): +- self.batch.minions = {"foo", "bar"} +- self.batch.batch_size = 2 +- self.assertEqual(self.batch._get_next(), {"foo", "bar"}) +- +- def test_next_batch_one_done(self): +- self.batch.minions = {"foo", "bar"} +- self.batch.done_minions = {"bar"} +- self.batch.batch_size = 2 +- self.assertEqual(self.batch._get_next(), {"foo"}) +- +- def test_next_batch_one_done_one_active(self): +- self.batch.minions = {"foo", "bar", "baz"} +- self.batch.done_minions = {"bar"} +- self.batch.active = {"baz"} +- self.batch.batch_size = 2 +- self.assertEqual(self.batch._get_next(), {"foo"}) +- +- def test_next_batch_one_done_one_active_one_timedout(self): +- self.batch.minions = {"foo", "bar", "baz", "faz"} +- self.batch.done_minions = {"bar"} +- self.batch.active = {"baz"} +- self.batch.timedout_minions = {"faz"} +- self.batch.batch_size = 2 +- self.assertEqual(self.batch._get_next(), {"foo"}) +- +- def test_next_batch_bigger_size(self): +- self.batch.minions = {"foo", "bar"} +- self.batch.batch_size = 3 +- self.assertEqual(self.batch._get_next(), {"foo", "bar"}) +- +- def test_next_batch_all_done(self): +- self.batch.minions = {"foo", "bar"} +- self.batch.done_minions = {"foo", "bar"} +- self.batch.batch_size = 2 +- self.assertEqual(self.batch._get_next(), set()) +- +- def test_next_batch_all_active(self): +- self.batch.minions = {"foo", "bar"} +- self.batch.active = {"foo", "bar"} +- self.batch.batch_size = 2 +- self.assertEqual(self.batch._get_next(), set()) +- +- def test_next_batch_all_timedout(self): +- self.batch.minions = {"foo", "bar"} +- self.batch.timedout_minions = {"foo", "bar"} +- self.batch.batch_size = 2 +- self.assertEqual(self.batch._get_next(), set()) +- +- def test_batch__event_handler_ping_return(self): +- self.batch.targeted_minions = {"foo"} +- self.batch.event = MagicMock( +- unpack=MagicMock(return_value=("salt/job/1234/ret/foo", {"id": "foo"})) +- ) +- self.batch.start() +- self.assertEqual(self.batch.minions, set()) +- self.batch._BatchAsync__event_handler(MagicMock()) +- self.assertEqual(self.batch.minions, {"foo"}) +- self.assertEqual(self.batch.done_minions, set()) +- +- def test_batch__event_handler_call_start_batch_when_all_pings_return(self): +- self.batch.targeted_minions = {"foo"} +- self.batch.event = MagicMock( +- unpack=MagicMock(return_value=("salt/job/1234/ret/foo", {"id": "foo"})) +- ) +- self.batch.start() +- self.batch._BatchAsync__event_handler(MagicMock()) +- self.assertEqual( +- self.batch.event.io_loop.spawn_callback.call_args[0], +- (self.batch.start_batch,), +- ) + +- def test_batch__event_handler_not_call_start_batch_when_not_all_pings_return(self): +- self.batch.targeted_minions = {"foo", "bar"} +- self.batch.event = MagicMock( +- unpack=MagicMock(return_value=("salt/job/1234/ret/foo", {"id": "foo"})) +- ) +- self.batch.start() +- self.batch._BatchAsync__event_handler(MagicMock()) +- self.assertEqual(len(self.batch.event.io_loop.spawn_callback.mock_calls), 0) ++def test_batch_jid(batch): ++ assert batch.batch_jid == "1235" ++ ++ ++def test_find_job_jid(batch): ++ assert batch.find_job_jid == "1236" ++ + +- def test_batch__event_handler_batch_run_return(self): +- self.batch.event = MagicMock( +- unpack=MagicMock(return_value=("salt/job/1235/ret/foo", {"id": "foo"})) ++def test_batch_size(batch): ++ """ ++ Tests passing batch value as a number ++ """ ++ batch.opts = {"batch": "2", "timeout": 5} ++ batch.minions = {"foo", "bar"} ++ batch.start_batch() ++ assert batch.batch_size == 2 ++ ++ ++def test_batch_start_on_batch_presence_ping_timeout(batch): ++ # batch_async = BatchAsyncMock(); ++ batch.event = MagicMock() ++ future = salt.ext.tornado.gen.Future() ++ future.set_result({"minions": ["foo", "bar"]}) ++ batch.local.run_job_async.return_value = future ++ with patch("salt.ext.tornado.gen.sleep", return_value=future): ++ # ret = batch_async.start(batch) ++ ret = batch.start() ++ # assert start_batch is called later with batch_presence_ping_timeout as param ++ assert batch.event.io_loop.spawn_callback.call_args[0] == (batch.start_batch,) ++ # assert test.ping called ++ assert batch.local.run_job_async.call_args[0] == ("*", "test.ping", [], "glob") ++ # assert targeted_minions == all minions matched by tgt ++ assert batch.targeted_minions == {"foo", "bar"} ++ ++ ++def test_batch_start_on_gather_job_timeout(batch): ++ # batch_async = BatchAsyncMock(); ++ batch.event = MagicMock() ++ future = salt.ext.tornado.gen.Future() ++ future.set_result({"minions": ["foo", "bar"]}) ++ batch.local.run_job_async.return_value = future ++ batch.batch_presence_ping_timeout = None ++ with patch("salt.ext.tornado.gen.sleep", return_value=future): ++ # ret = batch_async.start(batch) ++ ret = batch.start() ++ # assert start_batch is called later with gather_job_timeout as param ++ assert batch.event.io_loop.spawn_callback.call_args[0] == (batch.start_batch,) ++ ++ ++def test_batch_fire_start_event(batch): ++ batch.minions = {"foo", "bar"} ++ batch.opts = {"batch": "2", "timeout": 5} ++ batch.event = MagicMock() ++ batch.metadata = {"mykey": "myvalue"} ++ batch.start_batch() ++ assert batch.event.fire_event.call_args[0] == ( ++ { ++ "available_minions": {"foo", "bar"}, ++ "down_minions": set(), ++ "metadata": batch.metadata, ++ }, ++ "salt/batch/1235/start", ++ ) ++ ++ ++def test_start_batch_calls_next(batch): ++ batch.run_next = MagicMock(return_value=MagicMock()) ++ batch.event = MagicMock() ++ batch.start_batch() ++ assert batch.initialized ++ assert batch.event.io_loop.spawn_callback.call_args[0] == (batch.run_next,) ++ ++ ++def test_batch_fire_done_event(batch): ++ batch.targeted_minions = {"foo", "baz", "bar"} ++ batch.minions = {"foo", "bar"} ++ batch.done_minions = {"foo"} ++ batch.timedout_minions = {"bar"} ++ batch.event = MagicMock() ++ batch.metadata = {"mykey": "myvalue"} ++ old_event = batch.event ++ batch.end_batch() ++ assert old_event.fire_event.call_args[0] == ( ++ { ++ "available_minions": {"foo", "bar"}, ++ "done_minions": batch.done_minions, ++ "down_minions": {"baz"}, ++ "timedout_minions": batch.timedout_minions, ++ "metadata": batch.metadata, ++ }, ++ "salt/batch/1235/done", ++ ) ++ ++ ++def test_batch__del__(batch): ++ batch = BatchAsync(MagicMock(), MagicMock(), MagicMock()) ++ event = MagicMock() ++ batch.event = event ++ batch.__del__() ++ assert batch.local is None ++ assert batch.event is None ++ assert batch.ioloop is None ++ ++ ++def test_batch_close_safe(batch): ++ batch = BatchAsync(MagicMock(), MagicMock(), MagicMock()) ++ event = MagicMock() ++ batch.event = event ++ batch.patterns = { ++ ("salt/job/1234/ret/*", "find_job_return"), ++ ("salt/job/4321/ret/*", "find_job_return"), ++ } ++ batch.close_safe() ++ assert batch.local is None ++ assert batch.event is None ++ assert batch.ioloop is None ++ assert len(event.unsubscribe.mock_calls) == 2 ++ assert len(event.remove_event_handler.mock_calls) == 1 ++ ++ ++def test_batch_next(batch): ++ batch.event = MagicMock() ++ batch.opts["fun"] = "my.fun" ++ batch.opts["arg"] = [] ++ batch._get_next = MagicMock(return_value={"foo", "bar"}) ++ batch.batch_size = 2 ++ future = salt.ext.tornado.gen.Future() ++ future.set_result({"minions": ["foo", "bar"]}) ++ batch.local.run_job_async.return_value = future ++ with patch("salt.ext.tornado.gen.sleep", return_value=future): ++ batch.run_next() ++ assert batch.local.run_job_async.call_args[0] == ( ++ {"foo", "bar"}, ++ "my.fun", ++ [], ++ "list", + ) +- self.batch.start() +- self.batch.active = {"foo"} +- self.batch._BatchAsync__event_handler(MagicMock()) +- self.assertEqual(self.batch.active, set()) +- self.assertEqual(self.batch.done_minions, {"foo"}) +- self.assertEqual( +- self.batch.event.io_loop.spawn_callback.call_args[0], +- (self.batch.schedule_next,), ++ assert batch.event.io_loop.spawn_callback.call_args[0] == ( ++ batch.find_job, ++ {"foo", "bar"}, + ) ++ assert batch.active == {"bar", "foo"} ++ + +- def test_batch__event_handler_find_job_return(self): +- self.batch.event = MagicMock( +- unpack=MagicMock( +- return_value=( +- "salt/job/1236/ret/foo", +- {"id": "foo", "return": "deadbeaf"}, +- ) ++def test_next_batch(batch): ++ batch.minions = {"foo", "bar"} ++ batch.batch_size = 2 ++ assert batch._get_next() == {"foo", "bar"} ++ ++ ++def test_next_batch_one_done(batch): ++ batch.minions = {"foo", "bar"} ++ batch.done_minions = {"bar"} ++ batch.batch_size = 2 ++ assert batch._get_next() == {"foo"} ++ ++ ++def test_next_batch_one_done_one_active(batch): ++ batch.minions = {"foo", "bar", "baz"} ++ batch.done_minions = {"bar"} ++ batch.active = {"baz"} ++ batch.batch_size = 2 ++ assert batch._get_next() == {"foo"} ++ ++ ++def test_next_batch_one_done_one_active_one_timedout(batch): ++ batch.minions = {"foo", "bar", "baz", "faz"} ++ batch.done_minions = {"bar"} ++ batch.active = {"baz"} ++ batch.timedout_minions = {"faz"} ++ batch.batch_size = 2 ++ assert batch._get_next() == {"foo"} ++ ++ ++def test_next_batch_bigger_size(batch): ++ batch.minions = {"foo", "bar"} ++ batch.batch_size = 3 ++ assert batch._get_next() == {"foo", "bar"} ++ ++ ++def test_next_batch_all_done(batch): ++ batch.minions = {"foo", "bar"} ++ batch.done_minions = {"foo", "bar"} ++ batch.batch_size = 2 ++ assert batch._get_next() == set() ++ ++ ++def test_next_batch_all_active(batch): ++ batch.minions = {"foo", "bar"} ++ batch.active = {"foo", "bar"} ++ batch.batch_size = 2 ++ assert batch._get_next() == set() ++ ++ ++def test_next_batch_all_timedout(batch): ++ batch.minions = {"foo", "bar"} ++ batch.timedout_minions = {"foo", "bar"} ++ batch.batch_size = 2 ++ assert batch._get_next() == set() ++ ++ ++def test_batch__event_handler_ping_return(batch): ++ batch.targeted_minions = {"foo"} ++ batch.event = MagicMock( ++ unpack=MagicMock(return_value=("salt/job/1234/ret/foo", {"id": "foo"})) ++ ) ++ batch.start() ++ assert batch.minions == set() ++ batch._BatchAsync__event_handler(MagicMock()) ++ assert batch.minions == {"foo"} ++ assert batch.done_minions == set() ++ ++ ++def test_batch__event_handler_call_start_batch_when_all_pings_return(batch): ++ batch.targeted_minions = {"foo"} ++ batch.event = MagicMock( ++ unpack=MagicMock(return_value=("salt/job/1234/ret/foo", {"id": "foo"})) ++ ) ++ batch.start() ++ batch._BatchAsync__event_handler(MagicMock()) ++ assert batch.event.io_loop.spawn_callback.call_args[0] == (batch.start_batch,) ++ ++ ++def test_batch__event_handler_not_call_start_batch_when_not_all_pings_return(batch): ++ batch.targeted_minions = {"foo", "bar"} ++ batch.event = MagicMock( ++ unpack=MagicMock(return_value=("salt/job/1234/ret/foo", {"id": "foo"})) ++ ) ++ batch.start() ++ batch._BatchAsync__event_handler(MagicMock()) ++ assert len(batch.event.io_loop.spawn_callback.mock_calls) == 0 ++ ++ ++def test_batch__event_handler_batch_run_return(batch): ++ batch.event = MagicMock( ++ unpack=MagicMock(return_value=("salt/job/1235/ret/foo", {"id": "foo"})) ++ ) ++ batch.start() ++ batch.active = {"foo"} ++ batch._BatchAsync__event_handler(MagicMock()) ++ assert batch.active == set() ++ assert batch.done_minions == {"foo"} ++ assert batch.event.io_loop.spawn_callback.call_args[0] == (batch.schedule_next,) ++ ++ ++def test_batch__event_handler_find_job_return(batch): ++ batch.event = MagicMock( ++ unpack=MagicMock( ++ return_value=( ++ "salt/job/1236/ret/foo", ++ {"id": "foo", "return": "deadbeaf"}, + ) + ) +- self.batch.start() +- self.batch.patterns.add(("salt/job/1236/ret/*", "find_job_return")) +- self.batch._BatchAsync__event_handler(MagicMock()) +- self.assertEqual(self.batch.find_job_returned, {"foo"}) +- +- @salt.ext.tornado.testing.gen_test +- def test_batch_run_next_end_batch_when_no_next(self): +- self.batch.end_batch = MagicMock() +- self.batch._get_next = MagicMock(return_value={}) +- self.batch.run_next() +- self.assertEqual(len(self.batch.end_batch.mock_calls), 1) +- +- @salt.ext.tornado.testing.gen_test +- def test_batch_find_job(self): +- self.batch.event = MagicMock() +- future = salt.ext.tornado.gen.Future() +- future.set_result({}) +- self.batch.local.run_job_async.return_value = future +- self.batch.minions = {"foo", "bar"} +- self.batch.jid_gen = MagicMock(return_value="1234") +- salt.ext.tornado.gen.sleep = MagicMock(return_value=future) +- self.batch.find_job({"foo", "bar"}) +- self.assertEqual( +- self.batch.event.io_loop.spawn_callback.call_args[0], +- (self.batch.check_find_job, {"foo", "bar"}, "1234"), ++ ) ++ batch.start() ++ batch.patterns.add(("salt/job/1236/ret/*", "find_job_return")) ++ batch._BatchAsync__event_handler(MagicMock()) ++ assert batch.find_job_returned == {"foo"} ++ ++ ++def test_batch_run_next_end_batch_when_no_next(batch): ++ batch.end_batch = MagicMock() ++ batch._get_next = MagicMock(return_value={}) ++ batch.run_next() ++ assert len(batch.end_batch.mock_calls) == 1 ++ ++ ++def test_batch_find_job(batch): ++ batch.event = MagicMock() ++ future = salt.ext.tornado.gen.Future() ++ future.set_result({}) ++ batch.local.run_job_async.return_value = future ++ batch.minions = {"foo", "bar"} ++ batch.jid_gen = MagicMock(return_value="1234") ++ with patch("salt.ext.tornado.gen.sleep", return_value=future): ++ batch.find_job({"foo", "bar"}) ++ assert batch.event.io_loop.spawn_callback.call_args[0] == ( ++ batch.check_find_job, ++ {"foo", "bar"}, ++ "1234", + ) + +- @salt.ext.tornado.testing.gen_test +- def test_batch_find_job_with_done_minions(self): +- self.batch.done_minions = {"bar"} +- self.batch.event = MagicMock() +- future = salt.ext.tornado.gen.Future() +- future.set_result({}) +- self.batch.local.run_job_async.return_value = future +- self.batch.minions = {"foo", "bar"} +- self.batch.jid_gen = MagicMock(return_value="1234") +- salt.ext.tornado.gen.sleep = MagicMock(return_value=future) +- self.batch.find_job({"foo", "bar"}) +- self.assertEqual( +- self.batch.event.io_loop.spawn_callback.call_args[0], +- (self.batch.check_find_job, {"foo"}, "1234"), +- ) + +- def test_batch_check_find_job_did_not_return(self): +- self.batch.event = MagicMock() +- self.batch.active = {"foo"} +- self.batch.find_job_returned = set() +- self.batch.patterns = {("salt/job/1234/ret/*", "find_job_return")} +- self.batch.check_find_job({"foo"}, jid="1234") +- self.assertEqual(self.batch.find_job_returned, set()) +- self.assertEqual(self.batch.active, set()) +- self.assertEqual(len(self.batch.event.io_loop.add_callback.mock_calls), 0) +- +- def test_batch_check_find_job_did_return(self): +- self.batch.event = MagicMock() +- self.batch.find_job_returned = {"foo"} +- self.batch.patterns = {("salt/job/1234/ret/*", "find_job_return")} +- self.batch.check_find_job({"foo"}, jid="1234") +- self.assertEqual( +- self.batch.event.io_loop.spawn_callback.call_args[0], +- (self.batch.find_job, {"foo"}), ++def test_batch_find_job_with_done_minions(batch): ++ batch.done_minions = {"bar"} ++ batch.event = MagicMock() ++ future = salt.ext.tornado.gen.Future() ++ future.set_result({}) ++ batch.local.run_job_async.return_value = future ++ batch.minions = {"foo", "bar"} ++ batch.jid_gen = MagicMock(return_value="1234") ++ with patch("salt.ext.tornado.gen.sleep", return_value=future): ++ batch.find_job({"foo", "bar"}) ++ assert batch.event.io_loop.spawn_callback.call_args[0] == ( ++ batch.check_find_job, ++ {"foo"}, ++ "1234", + ) + +- def test_batch_check_find_job_multiple_states(self): +- self.batch.event = MagicMock() +- # currently running minions +- self.batch.active = {"foo", "bar"} + +- # minion is running and find_job returns +- self.batch.find_job_returned = {"foo"} ++def test_batch_check_find_job_did_not_return(batch): ++ batch.event = MagicMock() ++ batch.active = {"foo"} ++ batch.find_job_returned = set() ++ batch.patterns = {("salt/job/1234/ret/*", "find_job_return")} ++ batch.check_find_job({"foo"}, jid="1234") ++ assert batch.find_job_returned == set() ++ assert batch.active == set() ++ assert len(batch.event.io_loop.add_callback.mock_calls) == 0 + +- # minion started running but find_job did not return +- self.batch.timedout_minions = {"faz"} + +- # minion finished +- self.batch.done_minions = {"baz"} ++def test_batch_check_find_job_did_return(batch): ++ batch.event = MagicMock() ++ batch.find_job_returned = {"foo"} ++ batch.patterns = {("salt/job/1234/ret/*", "find_job_return")} ++ batch.check_find_job({"foo"}, jid="1234") ++ assert batch.event.io_loop.spawn_callback.call_args[0] == (batch.find_job, {"foo"}) + +- # both not yet done but only 'foo' responded to find_job +- not_done = {"foo", "bar"} + +- self.batch.patterns = {("salt/job/1234/ret/*", "find_job_return")} +- self.batch.check_find_job(not_done, jid="1234") ++def test_batch_check_find_job_multiple_states(batch): ++ batch.event = MagicMock() ++ # currently running minions ++ batch.active = {"foo", "bar"} + +- # assert 'bar' removed from active +- self.assertEqual(self.batch.active, {"foo"}) ++ # minion is running and find_job returns ++ batch.find_job_returned = {"foo"} + +- # assert 'bar' added to timedout_minions +- self.assertEqual(self.batch.timedout_minions, {"bar", "faz"}) ++ # minion started running but find_job did not return ++ batch.timedout_minions = {"faz"} ++ ++ # minion finished ++ batch.done_minions = {"baz"} ++ ++ # both not yet done but only 'foo' responded to find_job ++ not_done = {"foo", "bar"} ++ ++ batch.patterns = {("salt/job/1234/ret/*", "find_job_return")} ++ batch.check_find_job(not_done, jid="1234") ++ ++ # assert 'bar' removed from active ++ assert batch.active == {"foo"} ++ ++ # assert 'bar' added to timedout_minions ++ assert batch.timedout_minions == {"bar", "faz"} ++ ++ # assert 'find_job' schedueled again only for 'foo' ++ assert batch.event.io_loop.spawn_callback.call_args[0] == (batch.find_job, {"foo"}) + +- # assert 'find_job' schedueled again only for 'foo' +- self.assertEqual( +- self.batch.event.io_loop.spawn_callback.call_args[0], +- (self.batch.find_job, {"foo"}), +- ) + +- def test_only_on_run_next_is_scheduled(self): +- self.batch.event = MagicMock() +- self.batch.scheduled = True +- self.batch.schedule_next() +- self.assertEqual(len(self.batch.event.io_loop.spawn_callback.mock_calls), 0) ++def test_only_on_run_next_is_scheduled(batch): ++ batch.event = MagicMock() ++ batch.scheduled = True ++ batch.schedule_next() ++ assert len(batch.event.io_loop.spawn_callback.mock_calls) == 0 +diff --git a/tests/unit/cli/test_support.py b/tests/unit/cli/test_support.py +index dc0e99bb3d..971a0f122b 100644 +--- a/tests/unit/cli/test_support.py ++++ b/tests/unit/cli/test_support.py +@@ -14,7 +14,7 @@ from salt.cli.support.collector import SaltSupport, SupportDataCollector + from salt.cli.support.console import IndentOutput + from salt.utils.color import get_colors + from salt.utils.stringutils import to_bytes +-from tests.support.mock import NO_MOCK, NO_MOCK_REASON, MagicMock, patch ++from tests.support.mock import MagicMock, patch + from tests.support.unit import TestCase, skipIf + + try: +@@ -24,7 +24,6 @@ except ImportError: + + + @skipIf(not bool(pytest), "Pytest needs to be installed") +-@skipIf(NO_MOCK, NO_MOCK_REASON) + class SaltSupportIndentOutputTestCase(TestCase): + """ + Unit Tests for the salt-support indent output. +@@ -100,7 +99,6 @@ class SaltSupportIndentOutputTestCase(TestCase): + + + @skipIf(not bool(pytest), "Pytest needs to be installed") +-@skipIf(NO_MOCK, NO_MOCK_REASON) + class SaltSupportCollectorTestCase(TestCase): + """ + Collector tests. +@@ -232,7 +230,6 @@ class SaltSupportCollectorTestCase(TestCase): + + + @skipIf(not bool(pytest), "Pytest needs to be installed") +-@skipIf(NO_MOCK, NO_MOCK_REASON) + class SaltSupportRunnerTestCase(TestCase): + """ + Test runner class. +@@ -468,7 +465,6 @@ class SaltSupportRunnerTestCase(TestCase): + + + @skipIf(not bool(pytest), "Pytest needs to be installed") +-@skipIf(NO_MOCK, NO_MOCK_REASON) + class ProfileIntegrityTestCase(TestCase): + """ + Default profile integrity +diff --git a/tests/unit/modules/test_saltsupport.py b/tests/unit/modules/test_saltsupport.py +index 1715c68f4c..2afdd69b3e 100644 +--- a/tests/unit/modules/test_saltsupport.py ++++ b/tests/unit/modules/test_saltsupport.py +@@ -8,7 +8,7 @@ import datetime + import salt.exceptions + from salt.modules import saltsupport + from tests.support.mixins import LoaderModuleMockMixin +-from tests.support.mock import NO_MOCK, NO_MOCK_REASON, MagicMock, patch ++from tests.support.mock import MagicMock, patch + from tests.support.unit import TestCase, skipIf + + try: +@@ -18,7 +18,6 @@ except ImportError: + + + @skipIf(not bool(pytest), "Pytest required") +-@skipIf(NO_MOCK, NO_MOCK_REASON) + class SaltSupportModuleTestCase(TestCase, LoaderModuleMockMixin): + """ + Test cases for salt.modules.support::SaltSupportModule +@@ -361,7 +360,6 @@ professor: Farnsworth + + + @skipIf(not bool(pytest), "Pytest required") +-@skipIf(NO_MOCK, NO_MOCK_REASON) + class LogCollectorTestCase(TestCase, LoaderModuleMockMixin): + """ + Test cases for salt.modules.support::LogCollector +-- +2.41.0 + diff --git a/make-sure-configured-user-is-properly-set-by-salt-bs.patch b/make-sure-configured-user-is-properly-set-by-salt-bs.patch new file mode 100644 index 0000000..4702068 --- /dev/null +++ b/make-sure-configured-user-is-properly-set-by-salt-bs.patch @@ -0,0 +1,204 @@ +From 5ea4add5c8e2bed50b9825edfff7565e5f6124f3 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?= + +Date: Tue, 22 Aug 2023 12:57:44 +0100 +Subject: [PATCH] Make sure configured user is properly set by Salt + (bsc#1210994) (#596) + +* Make sure Salt user and env is validated before daemon init + +* Ensure HOME is always present in env and set according to pwuser + +* Set User to salt in salt-master.service files + +* Return proper exitcode if user is not valid + +* Fix environment also for salt-ssh command + +* Increase start_timeout to avoid test to be flaky +--- + pkg/common/salt-master.service | 1 + + pkg/old/deb/salt-master.service | 1 + + pkg/old/suse/salt-master.service | 1 + + salt/cli/daemons.py | 27 +++++++++++++++++++ + salt/cli/ssh.py | 8 ++++++ + salt/utils/verify.py | 4 +-- + .../integration/cli/test_salt_minion.py | 4 +-- + 7 files changed, 42 insertions(+), 4 deletions(-) + +diff --git a/pkg/common/salt-master.service b/pkg/common/salt-master.service +index 377c87afeb..257ecc283f 100644 +--- a/pkg/common/salt-master.service ++++ b/pkg/common/salt-master.service +@@ -8,6 +8,7 @@ LimitNOFILE=100000 + Type=notify + NotifyAccess=all + ExecStart=/usr/bin/salt-master ++User=salt + + [Install] + WantedBy=multi-user.target +diff --git a/pkg/old/deb/salt-master.service b/pkg/old/deb/salt-master.service +index b5d0cdd22c..f9dca296b4 100644 +--- a/pkg/old/deb/salt-master.service ++++ b/pkg/old/deb/salt-master.service +@@ -7,6 +7,7 @@ LimitNOFILE=16384 + Type=notify + NotifyAccess=all + ExecStart=/usr/bin/salt-master ++User=salt + + [Install] + WantedBy=multi-user.target +diff --git a/pkg/old/suse/salt-master.service b/pkg/old/suse/salt-master.service +index 9e002d16ca..caabca511c 100644 +--- a/pkg/old/suse/salt-master.service ++++ b/pkg/old/suse/salt-master.service +@@ -8,6 +8,7 @@ LimitNOFILE=100000 + Type=simple + ExecStart=/usr/bin/salt-master + TasksMax=infinity ++User=salt + + [Install] + WantedBy=multi-user.target +diff --git a/salt/cli/daemons.py b/salt/cli/daemons.py +index ecc05c919e..c9ee9ced91 100644 +--- a/salt/cli/daemons.py ++++ b/salt/cli/daemons.py +@@ -7,6 +7,7 @@ import logging + import os + import warnings + ++import salt.defaults.exitcodes + import salt.utils.kinds as kinds + from salt.exceptions import SaltClientError, SaltSystemExit, get_error_message + from salt.utils import migrations +@@ -73,6 +74,16 @@ class DaemonsMixin: # pylint: disable=no-init + self.__class__.__name__, + ) + ++ def verify_user(self): ++ """ ++ Verify Salt configured user for Salt and shutdown daemon if not valid. ++ ++ :return: ++ """ ++ if not check_user(self.config["user"]): ++ self.action_log_info("Cannot switch to configured user for Salt. Exiting") ++ self.shutdown(salt.defaults.exitcodes.EX_NOUSER) ++ + def action_log_info(self, action): + """ + Say daemon starting. +@@ -178,6 +189,10 @@ class Master( + self.config["interface"] = ip_bracket(self.config["interface"]) + migrations.migrate_paths(self.config) + ++ # Ensure configured user is valid and environment is properly set ++ # before initializating rest of the stack. ++ self.verify_user() ++ + # Late import so logging works correctly + import salt.master + +@@ -290,6 +305,10 @@ class Minion( + + transport = self.config.get("transport").lower() + ++ # Ensure configured user is valid and environment is properly set ++ # before initializating rest of the stack. ++ self.verify_user() ++ + try: + # Late import so logging works correctly + import salt.minion +@@ -478,6 +497,10 @@ class ProxyMinion( + self.action_log_info("An instance is already running. Exiting") + self.shutdown(1) + ++ # Ensure configured user is valid and environment is properly set ++ # before initializating rest of the stack. ++ self.verify_user() ++ + # TODO: AIO core is separate from transport + # Late import so logging works correctly + import salt.minion +@@ -576,6 +599,10 @@ class Syndic( + + self.action_log_info('Setting up "{}"'.format(self.config["id"])) + ++ # Ensure configured user is valid and environment is properly set ++ # before initializating rest of the stack. ++ self.verify_user() ++ + # Late import so logging works correctly + import salt.minion + +diff --git a/salt/cli/ssh.py b/salt/cli/ssh.py +index 6048cb5f58..672f32b8c0 100644 +--- a/salt/cli/ssh.py ++++ b/salt/cli/ssh.py +@@ -1,7 +1,9 @@ + import sys + + import salt.client.ssh ++import salt.defaults.exitcodes + import salt.utils.parsers ++from salt.utils.verify import check_user + + + class SaltSSH(salt.utils.parsers.SaltSSHOptionParser): +@@ -15,5 +17,11 @@ class SaltSSH(salt.utils.parsers.SaltSSHOptionParser): + # that won't be used anyways with -H or --hosts + self.parse_args() + ++ if not check_user(self.config["user"]): ++ self.exit( ++ salt.defaults.exitcodes.EX_NOUSER, ++ "Cannot switch to configured user for Salt. Exiting", ++ ) ++ + ssh = salt.client.ssh.SSH(self.config) + ssh.run() +diff --git a/salt/utils/verify.py b/salt/utils/verify.py +index 879128f231..7899fbe538 100644 +--- a/salt/utils/verify.py ++++ b/salt/utils/verify.py +@@ -335,8 +335,8 @@ def check_user(user): + + # We could just reset the whole environment but let's just override + # the variables we can get from pwuser +- if "HOME" in os.environ: +- os.environ["HOME"] = pwuser.pw_dir ++ # We ensure HOME is always present and set according to pwuser ++ os.environ["HOME"] = pwuser.pw_dir + + if "SHELL" in os.environ: + os.environ["SHELL"] = pwuser.pw_shell +diff --git a/tests/pytests/integration/cli/test_salt_minion.py b/tests/pytests/integration/cli/test_salt_minion.py +index c0d6013474..bde2dd51d7 100644 +--- a/tests/pytests/integration/cli/test_salt_minion.py ++++ b/tests/pytests/integration/cli/test_salt_minion.py +@@ -41,7 +41,7 @@ def test_exit_status_unknown_user(salt_master, minion_id): + factory = salt_master.salt_minion_daemon( + minion_id, overrides={"user": "unknown-user"} + ) +- factory.start(start_timeout=10, max_start_attempts=1) ++ factory.start(start_timeout=30, max_start_attempts=1) + + assert exc.value.process_result.returncode == salt.defaults.exitcodes.EX_NOUSER + assert "The user is not available." in exc.value.process_result.stderr +@@ -53,7 +53,7 @@ def test_exit_status_unknown_argument(salt_master, minion_id): + """ + with pytest.raises(FactoryNotStarted) as exc: + factory = salt_master.salt_minion_daemon(minion_id) +- factory.start("--unknown-argument", start_timeout=10, max_start_attempts=1) ++ factory.start("--unknown-argument", start_timeout=30, max_start_attempts=1) + + assert exc.value.process_result.returncode == salt.defaults.exitcodes.EX_USAGE + assert "Usage" in exc.value.process_result.stderr +-- +2.41.0 + + diff --git a/prevent-possible-exceptions-on-salt.utils.user.get_g.patch b/prevent-possible-exceptions-on-salt.utils.user.get_g.patch new file mode 100644 index 0000000..aa0f024 --- /dev/null +++ b/prevent-possible-exceptions-on-salt.utils.user.get_g.patch @@ -0,0 +1,68 @@ +From 4ea91a61abbb6ef001f057685370454c85b72c3a Mon Sep 17 00:00:00 2001 +From: Victor Zhestkov +Date: Mon, 21 Aug 2023 13:04:32 +0200 +Subject: [PATCH] Prevent possible exceptions on + salt.utils.user.get_group_dict (bsc#1212794) + +* Prevent KeyError on calling grp.getgrnam in case of improper group + +* Add test of calling salt.utils.user.get_group_dict + +for the user having improper duplicate group + +* Update tests/pytests/functional/utils/user/test_get_group_dict.py + +Co-authored-by: Pedro Algarvio + +--------- + +Co-authored-by: Pedro Algarvio +--- + salt/utils/user.py | 6 +++++- + .../utils/user/test_get_group_dict.py | 17 +++++++++++++++++ + 2 files changed, 22 insertions(+), 1 deletion(-) + create mode 100644 tests/pytests/functional/utils/user/test_get_group_dict.py + +diff --git a/salt/utils/user.py b/salt/utils/user.py +index 9763667443..2f1ca65cf9 100644 +--- a/salt/utils/user.py ++++ b/salt/utils/user.py +@@ -352,7 +352,11 @@ def get_group_dict(user=None, include_default=True): + group_dict = {} + group_names = get_group_list(user, include_default=include_default) + for group in group_names: +- group_dict.update({group: grp.getgrnam(group).gr_gid}) ++ try: ++ group_dict.update({group: grp.getgrnam(group).gr_gid}) ++ except KeyError: ++ # In case if imporer duplicate group was returned by get_group_list ++ pass + return group_dict + + +diff --git a/tests/pytests/functional/utils/user/test_get_group_dict.py b/tests/pytests/functional/utils/user/test_get_group_dict.py +new file mode 100644 +index 0000000000..653d664607 +--- /dev/null ++++ b/tests/pytests/functional/utils/user/test_get_group_dict.py +@@ -0,0 +1,17 @@ ++import logging ++ ++import pytest ++ ++import salt.utils.platform ++import salt.utils.user ++from tests.support.mock import patch ++ ++log = logging.getLogger(__name__) ++ ++pytestmark = [ ++ pytest.mark.skip_unless_on_linux(reason="Should only run in platforms which have duplicate GID support"), ++] ++def test_get_group_dict_with_improper_duplicate_root_group(): ++ with patch("salt.utils.user.get_group_list", return_value=["+", "root"]): ++ group_list = salt.utils.user.get_group_dict("root") ++ assert group_list == {"root": 0} +-- +2.41.0 + diff --git a/salt.changes b/salt.changes index 06c360d..7fa58c9 100644 --- a/salt.changes +++ b/salt.changes @@ -1,3 +1,17 @@ +------------------------------------------------------------------- +Tue Aug 22 12:03:21 UTC 2023 - Pablo Suárez Hernández + +- Make sure configured user is properly set by Salt (bsc#1210994) +- Do not fail on bad message pack message (bsc#1213441, CVE-2023-20897) +- Fix broken tests to make them running in the testsuite +- Prevent possible exceptions on salt.utils.user.get_group_dict (bsc#1212794) + +- Added: + * do-not-fail-on-bad-message-pack-message-bsc-1213441-.patch + * fix-tests-to-make-them-running-with-salt-testsuite.patch + * prevent-possible-exceptions-on-salt.utils.user.get_g.patch + * make-sure-configured-user-is-properly-set-by-salt-bs.patch + ------------------------------------------------------------------- Wed Aug 9 15:13:50 UTC 2023 - Alexander Graul diff --git a/salt.spec b/salt.spec index fe3aff9..46f2ded 100644 --- a/salt.spec +++ b/salt.spec @@ -289,6 +289,15 @@ Patch71: fix-the-regression-of-user.present-state-when-group-.patch Patch72: fix-regression-multiple-values-for-keyword-argument-.patch # PATCH-FIX_UPSTREAM: https://github.com/saltstack/salt/pull/64554 Patch73: mark-salt-3006-as-released-586.patch +# PATCH-FIX_UPSTREAM: https://github.com/saltstack/salt/pull/64599 +Patch74: prevent-possible-exceptions-on-salt.utils.user.get_g.patch +# PATCH-FIX_OPENSUSE: https://github.com/openSUSE/salt/pull/592 +Patch75: fix-tests-to-make-them-running-with-salt-testsuite.patch +# PATCH-FIX_UPSTREAM: https://github.com/saltstack/salt/commit/f82860b8ad3ee786762fa02fa1a6eaf6e24dc8d4 +# PATCH-FIX_UPSTREAM: https://github.com/saltstack/salt/pull/65020 +Patch76: do-not-fail-on-bad-message-pack-message-bsc-1213441-.patch +# PATCH-FIX_UPSTREAM: https://github.com/saltstack/salt/pull/64510 +Patch77: make-sure-configured-user-is-properly-set-by-salt-bs.patch ### IMPORTANT: The line below is used as a snippet marker. Do not touch it. ### SALT PATCHES LIST END