Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
systemsmanagement:Uyuni:Master:EL9-Uyuni-Client-Tools
venv-salt-minion
backport-syndic-auth-fixes.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File backport-syndic-auth-fixes.patch of Package venv-salt-minion
From 54ab69e74beb83710d0bf6049039d13e260d5517 Mon Sep 17 00:00:00 2001 From: Alexander Graul <agraul@suse.com> Date: Tue, 13 Sep 2022 11:26:21 +0200 Subject: [PATCH] Backport Syndic auth fixes [3004.2] Syndic Fixes (cherry picked from commit 643bd4b572ca97466e085ecd1d84da45b1684332) Co-authored-by: Megan Wilhite <megan.wilhite@gmail.com> --- changelog/61868.fixed | 1 + salt/transport/mixins/auth.py | 2 +- salt/transport/tcp.py | 2 +- salt/transport/zeromq.py | 2 +- tests/pytests/unit/transport/test_tcp.py | 149 +++++++++++++++++++- tests/pytests/unit/transport/test_zeromq.py | 73 +++++++++- 6 files changed, 224 insertions(+), 5 deletions(-) create mode 100644 changelog/61868.fixed diff --git a/changelog/61868.fixed b/changelog/61868.fixed new file mode 100644 index 0000000000..0169c48e99 --- /dev/null +++ b/changelog/61868.fixed @@ -0,0 +1 @@ +Make sure the correct key is being used when verifying or validating communication, eg. when a Salt syndic is involved use syndic_master.pub and when a Salt minion is involved use minion_master.pub. diff --git a/salt/transport/mixins/auth.py b/salt/transport/mixins/auth.py index 1e2e8e6b7b..e5c6a5345f 100644 --- a/salt/transport/mixins/auth.py +++ b/salt/transport/mixins/auth.py @@ -43,7 +43,7 @@ class AESPubClientMixin: ) # Verify that the signature is valid - master_pubkey_path = os.path.join(self.opts["pki_dir"], "minion_master.pub") + master_pubkey_path = os.path.join(self.opts["pki_dir"], self.auth.mpub) if not salt.crypt.verify_signature( master_pubkey_path, payload["load"], payload.get("sig") ): diff --git a/salt/transport/tcp.py b/salt/transport/tcp.py index f00b3c40eb..2821be82c7 100644 --- a/salt/transport/tcp.py +++ b/salt/transport/tcp.py @@ -295,7 +295,7 @@ class AsyncTCPReqChannel(salt.transport.client.ReqChannel): signed_msg = pcrypt.loads(ret[dictkey]) # Validate the master's signature. - master_pubkey_path = os.path.join(self.opts["pki_dir"], "minion_master.pub") + master_pubkey_path = os.path.join(self.opts["pki_dir"], self.auth.mpub) if not salt.crypt.verify_signature( master_pubkey_path, signed_msg["data"], signed_msg["sig"] ): diff --git a/salt/transport/zeromq.py b/salt/transport/zeromq.py index aa06298ee1..8199378239 100644 --- a/salt/transport/zeromq.py +++ b/salt/transport/zeromq.py @@ -255,7 +255,7 @@ class AsyncZeroMQReqChannel(salt.transport.client.ReqChannel): signed_msg = pcrypt.loads(ret[dictkey]) # Validate the master's signature. - master_pubkey_path = os.path.join(self.opts["pki_dir"], "minion_master.pub") + master_pubkey_path = os.path.join(self.opts["pki_dir"], self.auth.mpub) if not salt.crypt.verify_signature( master_pubkey_path, signed_msg["data"], signed_msg["sig"] ): diff --git a/tests/pytests/unit/transport/test_tcp.py b/tests/pytests/unit/transport/test_tcp.py index 3b6e175472..e41edcc37e 100644 --- a/tests/pytests/unit/transport/test_tcp.py +++ b/tests/pytests/unit/transport/test_tcp.py @@ -1,13 +1,53 @@ import contextlib +import os import socket import attr import pytest import salt.exceptions +import salt.transport.mixins.auth import salt.transport.tcp from salt.ext.tornado import concurrent, gen, ioloop from saltfactories.utils.ports import get_unused_localhost_port -from tests.support.mock import MagicMock, patch +from tests.support.mock import MagicMock, PropertyMock, create_autospec, patch + + +@pytest.fixture +def fake_keys(): + with patch("salt.crypt.AsyncAuth.get_keys", autospec=True): + yield + + +@pytest.fixture +def fake_crypto(): + with patch("salt.transport.tcp.PKCS1_OAEP", create=True) as fake_crypto: + yield fake_crypto + + +@pytest.fixture +def fake_authd(): + @salt.ext.tornado.gen.coroutine + def return_nothing(): + raise salt.ext.tornado.gen.Return() + + with patch( + "salt.crypt.AsyncAuth.authenticated", new_callable=PropertyMock + ) as mock_authed, patch( + "salt.crypt.AsyncAuth.authenticate", + autospec=True, + return_value=return_nothing(), + ), patch( + "salt.crypt.AsyncAuth.gen_token", autospec=True, return_value=42 + ): + mock_authed.return_value = False + yield + + +@pytest.fixture +def fake_crypticle(): + with patch("salt.crypt.Crypticle") as fake_crypticle: + fake_crypticle.generate_key_string.return_value = "fakey fake" + yield fake_crypticle @pytest.fixture @@ -405,3 +445,110 @@ def test_client_reconnect_backoff(client_socket): client.io_loop.run_sync(client._connect) finally: client.close() + + +async def test_when_async_req_channel_with_syndic_role_should_use_syndic_master_pub_file_to_verify_master_sig( + fake_keys, fake_crypto, fake_crypticle +): + # Syndics use the minion pki dir, but they also create a syndic_master.pub + # file for comms with the Salt master + expected_pubkey_path = os.path.join("/etc/salt/pki/minion", "syndic_master.pub") + fake_crypto.new.return_value.decrypt.return_value = "decrypted_return_value" + mockloop = MagicMock() + opts = { + "master_uri": "tcp://127.0.0.1:4506", + "interface": "127.0.0.1", + "ret_port": 4506, + "ipv6": False, + "sock_dir": ".", + "pki_dir": "/etc/salt/pki/minion", + "id": "syndic", + "__role": "syndic", + "keysize": 4096, + } + client = salt.transport.tcp.AsyncTCPReqChannel(opts, io_loop=mockloop) + + dictkey = "pillar" + target = "minion" + + # Mock auth and message client. + client.auth._authenticate_future = MagicMock() + client.auth._authenticate_future.done.return_value = True + client.auth._authenticate_future.exception.return_value = None + client.auth._crypticle = MagicMock() + client.message_client = create_autospec(client.message_client) + + @salt.ext.tornado.gen.coroutine + def mocksend(msg, timeout=60, tries=3): + raise salt.ext.tornado.gen.Return({"pillar": "data", "key": "value"}) + + client.message_client.send = mocksend + + # Note the 'ver' value in 'load' does not represent the the 'version' sent + # in the top level of the transport's message. + load = { + "id": target, + "grains": {}, + "saltenv": "base", + "pillarenv": "base", + "pillar_override": True, + "extra_minion_data": {}, + "ver": "2", + "cmd": "_pillar", + } + fake_nonce = 42 + with patch( + "salt.crypt.verify_signature", autospec=True, return_value=True + ) as fake_verify, patch( + "salt.payload.loads", + autospec=True, + return_value={"key": "value", "nonce": fake_nonce, "pillar": "data"}, + ), patch( + "uuid.uuid4", autospec=True + ) as fake_uuid: + fake_uuid.return_value.hex = fake_nonce + ret = await client.crypted_transfer_decode_dictentry( + load, + dictkey="pillar", + ) + + assert fake_verify.mock_calls[0].args[0] == expected_pubkey_path + + +async def test_mixin_should_use_correct_path_when_syndic( + fake_keys, fake_authd, fake_crypticle +): + mockloop = MagicMock() + expected_pubkey_path = os.path.join("/etc/salt/pki/minion", "syndic_master.pub") + opts = { + "master_uri": "tcp://127.0.0.1:4506", + "interface": "127.0.0.1", + "ret_port": 4506, + "ipv6": False, + "sock_dir": ".", + "pki_dir": "/etc/salt/pki/minion", + "id": "syndic", + "__role": "syndic", + "keysize": 4096, + "sign_pub_messages": True, + } + + with patch( + "salt.crypt.verify_signature", autospec=True, return_value=True + ) as fake_verify, patch( + "salt.utils.msgpack.loads", + autospec=True, + return_value={"enc": "aes", "load": "", "sig": "fake_signature"}, + ): + client = salt.transport.tcp.AsyncTCPPubChannel(opts, io_loop=mockloop) + client.message_client = MagicMock() + client.message_client.on_recv.side_effect = lambda x: x(b"some_data") + await client.connect() + client.auth._crypticle = fake_crypticle + + @client.on_recv + def test_recv_function(*args, **kwargs): + ... + + await test_recv_function + assert fake_verify.mock_calls[0].args[0] == expected_pubkey_path diff --git a/tests/pytests/unit/transport/test_zeromq.py b/tests/pytests/unit/transport/test_zeromq.py index 1f0515c91a..c3093f4b19 100644 --- a/tests/pytests/unit/transport/test_zeromq.py +++ b/tests/pytests/unit/transport/test_zeromq.py @@ -23,7 +23,7 @@ import salt.utils.process import salt.utils.stringutils from salt.master import SMaster from salt.transport.zeromq import AsyncReqMessageClientPool -from tests.support.mock import MagicMock, patch +from tests.support.mock import MagicMock, create_autospec, patch try: from M2Crypto import RSA @@ -608,6 +608,7 @@ async def test_req_chan_decode_data_dict_entry_v2(pki_dir): auth = client.auth auth._crypticle = salt.crypt.Crypticle(opts, AES_KEY) client.auth = MagicMock() + client.auth.mpub = auth.mpub client.auth.authenticated = True client.auth.get_keys = auth.get_keys client.auth.crypticle.dumps = auth.crypticle.dumps @@ -672,6 +673,7 @@ async def test_req_chan_decode_data_dict_entry_v2_bad_nonce(pki_dir): auth = client.auth auth._crypticle = salt.crypt.Crypticle(opts, AES_KEY) client.auth = MagicMock() + client.auth.mpub = auth.mpub client.auth.authenticated = True client.auth.get_keys = auth.get_keys client.auth.crypticle.dumps = auth.crypticle.dumps @@ -735,6 +737,7 @@ async def test_req_chan_decode_data_dict_entry_v2_bad_signature(pki_dir): auth = client.auth auth._crypticle = salt.crypt.Crypticle(opts, AES_KEY) client.auth = MagicMock() + client.auth.mpub = auth.mpub client.auth.authenticated = True client.auth.get_keys = auth.get_keys client.auth.crypticle.dumps = auth.crypticle.dumps @@ -814,6 +817,7 @@ async def test_req_chan_decode_data_dict_entry_v2_bad_key(pki_dir): auth = client.auth auth._crypticle = salt.crypt.Crypticle(opts, AES_KEY) client.auth = MagicMock() + client.auth.mpub = auth.mpub client.auth.authenticated = True client.auth.get_keys = auth.get_keys client.auth.crypticle.dumps = auth.crypticle.dumps @@ -1273,3 +1277,70 @@ 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_when_async_req_channel_with_syndic_role_should_use_syndic_master_pub_file_to_verify_master_sig( + pki_dir, +): + # Syndics use the minion pki dir, but they also create a syndic_master.pub + # file for comms with the Salt master + expected_pubkey_path = str(pki_dir.join("minion").join("syndic_master.pub")) + mockloop = MagicMock() + 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.join("minion")), + "id": "syndic", + "__role": "syndic", + "keysize": 4096, + } + master_opts = dict(opts, pki_dir=str(pki_dir.join("master"))) + server = salt.transport.zeromq.ZeroMQReqServerChannel(master_opts) + client = salt.transport.zeromq.AsyncZeroMQReqChannel(opts, io_loop=mockloop) + + dictkey = "pillar" + target = "minion" + pillar_data = {"pillar1": "data1"} + + # Mock auth and message client. + client.auth._authenticate_future = MagicMock() + client.auth._authenticate_future.done.return_value = True + client.auth._authenticate_future.exception.return_value = None + client.auth._crypticle = salt.crypt.Crypticle(opts, AES_KEY) + client.message_client = create_autospec(client.message_client) + + @salt.ext.tornado.gen.coroutine + def mocksend(msg, timeout=60, tries=3): + client.message_client.msg = msg + load = client.auth.crypticle.loads(msg["load"]) + ret = server._encrypt_private( + pillar_data, dictkey, target, nonce=load["nonce"], sign_messages=True + ) + raise salt.ext.tornado.gen.Return(ret) + + client.message_client.send = mocksend + + # Note the 'ver' value in 'load' does not represent the the 'version' sent + # in the top level of the transport's message. + load = { + "id": target, + "grains": {}, + "saltenv": "base", + "pillarenv": "base", + "pillar_override": True, + "extra_minion_data": {}, + "ver": "2", + "cmd": "_pillar", + } + with patch( + "salt.crypt.verify_signature", autospec=True, return_value=True + ) as fake_verify: + ret = await client.crypted_transfer_decode_dictentry( + load, + dictkey="pillar", + ) + + assert fake_verify.mock_calls[0].args[0] == expected_pubkey_path -- 2.37.3
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
